[SPARK-18929][ML] Add Tweedie distribution in GLM #16344

actuaryzhang · 2016-12-19T23:54:04Z

What changes were proposed in this pull request?

I propose to add the full Tweedie family into the GeneralizedLinearRegression model. The Tweedie family is characterized by a power variance function. Currently supported distributions such as Gaussian, Poisson and Gamma families are a special case of the Tweedie https://en.wikipedia.org/wiki/Tweedie_distribution.

@yanboliang @srowen @sethah

yanboliang · 2016-12-20T02:57:39Z

Jenkins, test this please.

srowen

That's probably reasonable. Doesn't this give us support for compound Poisson et al, by supporting general Tweedie distributions?

srowen · 2016-12-20T11:05:37Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

+      if (variancePower > 1.0 && variancePower < 2.0) {
+        require(y >= 0.0, "The response variable of the specified Tweedie distribution " +
+          s"should be non-negative, but got $y")
+        math.max(y, 0.1)


If we're going to use this magic 0.1 constant in many places, factor out a constant? 0.1 seems quite large as an 'epsilon' but I guess that's what R's implementation uses for whatever reason?

I have not seen a formal justification for the choice of 0.1 in R. This seminal paper (section 4) suggests 1/6 (about 0.17) to be the best constant. I would prefer to be consistent with R so that we can make comparison. Using a constant is a good idea.

srowen · 2016-12-20T11:06:00Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

+        deviance: Double,
+        numInstances: Double,
+        weightSum: Double): Double = {
+      0.0


Throw a UnsupportedOperationException?

srowen · 2016-12-20T11:07:39Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

+      if (mu < epsilon) {
+        epsilon
+      } else if (mu.isInfinity) {
+        Double.MaxValue


Out of curiosity is this meaningful to "cap" at Double.MaxValue? By the time you get there a lot of stuff is going to be infinite or not meaningful.

I see, it's done that way in other implementations. OK. I'm not sure if it's going to do much.

I think there's a problem in the Gaussian project method because it uses Double.MinValue to appear to mean "the smallest double" when it's the "smallest possible double" I'll investigate and file a bug if needed. EDIT: Ignore this side comment, Double.MinValue isn't the same as Double.MIN_VALUE in Java.

srowen · 2016-12-20T11:08:05Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

+
+    val defaultLink: Link = Log
+
+    var variancePower: Double = 1.5


This is a global shared variable -- we really can't do this.

Would you please suggest a better way to set the variancePower? I want to be consistent with the existing code to have the Family objects, but I need to also pass on the input variancePower to the Tweedie object which is used to compute the variance function. Any suggestion will be highly appreciated. @srowen @yanboliang

I think the Tweedie implementation needs to be able to access parameters of the GLM, to read off variancePower.

As it is this is a global variable and two jobs would overwrite each others' values.

srowen · 2016-12-20T17:58:07Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

@@ -242,7 +275,7 @@ class GeneralizedLinearRegression @Since("2.0.0") (@Since("2.0.0") override val
  def setLinkPredictionCol(value: String): this.type = set(linkPredictionCol, value)

  override protected def train(dataset: Dataset[_]): GeneralizedLinearRegressionModel = {
-    val familyObj = Family.fromName($(family))
+    val familyObj = Family.fromName($(family), $(variancePower))


I don't think we can do this either. variancePower is specific to one family, not a property of all of them.

srowen · 2016-12-20T17:59:17Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

+
+    val defaultLink: Link = Log
+
+    var variancePower: Double = 1.5


I think the Tweedie implementation needs to be able to access parameters of the GLM, to read off variancePower.

As it is this is a global variable and two jobs would overwrite each others' values.

…wer to varPower

actuaryzhang · 2016-12-20T23:09:18Z

@srowen Thanks for the comments. Really helpful. I have made a new commit that addresses the issues you raised:

I think the use of a global family object does not work well for the tweedie case, since we have to set the variance power. I now define a tweedie class and create the tweedie object within the train method of GeneralizedLinearRegression where the variance power is set. Does this make sense?
I created a constant delta = 0.1 in the Family class which is used to shift mu (or y) = 0 to avoid numerical issues.
I now change variancePower to varPower to save typing (and consistent with R and H2o).
The project method you asked about is copied from the other families, which I suppose is to bound the mean to stabilize estimation.
I now throw UnsupportedOperationException for tweedie AIC.

Let me know if there is any other issues.

Copying others for reviewing. @yanboliang @sethah

yanboliang · 2016-12-21T14:33:37Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

+   * @group param
+   */
+  @Since("2.2.0")
+  final val varPower: Param[Double] = new Param(this, "varPower",


I vote to revert this back to variancePower to follow MLlib's convention.

yanboliang · 2016-12-21T14:43:53Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

+   * Used only for the tweedie family.
+   * (see <a href="https://en.wikipedia.org/wiki/Tweedie_distribution">
+   * Tweedie Distribution (Wikipedia)</a>)
+   * Supported value: (1, 2) and (2, Inf).


Question: Why we don't allow 0, 1 and 2? They correspond respectively to Gaussian, Poisson and Gamma families, I think we should support fitting a poisson GLM via the tweedie family entrance and R can do it:

y <- rgamma(20,shape=5) x <- 1:20 glm(y~x,family=tweedie(var.power=1,link.power=1)) glm(y~x,family=poisson(link=identity))

yanboliang · 2016-12-21T15:02:55Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

-    Binomial -> Logit, Binomial -> Probit, Binomial -> CLogLog,
-    Poisson -> Log, Poisson -> Identity, Poisson -> Sqrt,
-    Gamma -> Inverse, Gamma -> Identity, Gamma -> Log
+    "gaussian" -> Identity, "gaussian" -> Log, "gaussian" -> Inverse,


String is error-prone, I think we can construct a member object for Tweedie whose variancePower is the default value(1.5).

@yanboliang The member object (in GeneralizedLinearRegressionBase) won't be accessible in Family, right? The method Family.fromName($(family)) uses global objects like Poisson, Gamma etc. To use Family.fromName, I need to create a Tweedie global object. Then we are back to the issue that @srowen pointed out of setting variancePower of the global object. Please advise. Thanks.

@yanboliang Could you help me understand the issue caused by using string? If I use object, then I have to create a Tweedie object that is not used anywhere else. And also I have to write two methods in Family: one returns the global Tweedie object (where the variancePower is preset) and one returns the a TweedieFamily object created using the user-specified variancePower. I hope we are fine using string since there are only a few values.

Yeah, you are right. It needs an extra global object to avoid error-prone which may a little expensive. I'm ok with using string.

srowen · 2016-12-21T11:21:57Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

+    "The power in the variance function of the Tweedie distribution which characterizes " +
+    "the relationship between the variance and mean of the distribution. " +
+    "Used only for the tweedie family. Supported value: (1, 2) and (2, Inf).",
+    (x: Double) => if (x > 1.0 && x != 2.0) true else false)


You can just write => x > 1.0 && x != 2.0. if (x) true else false is redundant.

srowen · 2016-12-21T11:22:28Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

@@ -64,6 +64,27 @@ private[regression] trait GeneralizedLinearRegressionBase extends PredictorParam
  def getFamily: String = $(family)

  /**
+   * Param for the power in the variance function of the Tweedie distribution which provides


Nits: Param -> parameter, tweedie -> Tweedie (two lines below).

changed tweedie. but other docs have been using Param..

srowen · 2016-12-21T11:23:50Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

@@ -397,14 +436,19 @@ object GeneralizedLinearRegression extends DefaultParamsReadable[GeneralizedLine

    /** Trim the fitted value so that it will be in valid range. */
    def project(mu: Double): Double = mu
+
+    /** Constant added to y = 0 for initialization or deviance to avoid numerical issues. */
+    val delta: Double = 0.1


This should be defined in an object; it's a static constant. The comment isn't quite accurate; it's not added, but it's a minimum.

srowen · 2016-12-21T11:25:11Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

@@ -242,7 +275,12 @@ class GeneralizedLinearRegression @Since("2.0.0") (@Since("2.0.0") override val
  def setLinkPredictionCol(value: String): this.type = set(linkPredictionCol, value)

  override protected def train(dataset: Dataset[_]): GeneralizedLinearRegressionModel = {
-    val familyObj = Family.fromName($(family))
+    val familyObj = if ($(family) == "tweedie") {
+      new Tweedie($(varPower))


Hm, why does this parameter need to be in the Family object at all? can't the implementation of Tweedie just go get the parameter's value? it's odd to have a Family representing all but one family, because Tweedie is one of them.

@srowen Family is not subclass of GeneralizedLinearRegression. Could you elaborate how to make it get the variancePower value?

Yes of course. Given how the code is structured, one straightforward solution is to generalize Family so that its operations take a reference to the model, so that implementation may access its parameters.

Another is to make the code instantiate Family subclasses instead of using single objects and give the instance a reference to the model.

srowen · 2016-12-21T11:25:23Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

     *
-     * @param name family name: "gaussian", "binomial", "poisson" or "gamma".
+     * @param name family name: "gaussian", "binomial", "poisson" and "gamma".


Nite: revert this, because it's really an "or"

srowen · 2016-12-21T15:13:34Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

+      if (mu < epsilon) {
+        epsilon
+      } else if (mu.isInfinity) {
+        Double.MaxValue


I see, it's done that way in other implementations. OK. I'm not sure if it's going to do much.

I think there's a problem in the Gaussian project method because it uses Double.MinValue to appear to mean "the smallest double" when it's the "smallest possible double" I'll investigate and file a bug if needed. EDIT: Ignore this side comment, Double.MinValue isn't the same as Double.MIN_VALUE in Java.

actuaryzhang · 2016-12-22T05:06:01Z

@srowen @yanboliang Thanks much for the feedback. I now have a better understanding of the code and the issue. I have made new commits reflecting your suggestions. The major changes are summarized below:

Create new method fromModel in the Family object to get the desired family object by accessing the GeneralizedLinearRegressionBase object directly. This solves the main challenge of setting the variancePower of the Tweedie family. The old fromName method is deleted since we now need both name and variancePower.
Create TweedieFamily as new subclass of family. This class encompasses the special cases of Gaussian, Poisson and Gamma, which can be handled together using the general formula based on variancePower.

Please let me know if there are any other issues. Thanks.

srowen

I think this is moving somewhere interesting, though I'm still not sure about this design. Comments inline.

srowen · 2016-12-22T12:04:20Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala


  /** Set of link names that GeneralizedLinearRegression supports. */
  private[regression] lazy val supportedLinkNames = supportedFamilyAndLinkPairs.map(_._2.name)

  private[regression] val epsilon: Double = 1E-16

+  /** Constant used in initialization and deviance to avoid numerical issues. */
+  private[regression] val delta: Double = 0.1


This should still be in an object IMHO; it's a constant right? epsilon really should be too. It's not a big deal but not quite right.

They are already in the GeneralizedLinearRegression object, aren't they? Or do you mean creating a new object say Constant that stores these two constants, and using them like Constant.delta?

Since delta is only used in the TweedieFamily class, I can also move it there. Let me know what is best. Thanks.

I'm OK to put it here.

Why not a companion object for TweedieFamily though? that just seems easy and more correct

srowen · 2016-12-22T12:11:53Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala


-    override def initialize(y: Double, weight: Double): Double = y
+    override def variance(mu: Double): Double = {


Instead of case statements like this, why not just override in subclasses?

srowen · 2016-12-22T12:12:18Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

-        weightSum: Double): Double = {
-      val wt = predictions.map(x => math.log(x._3)).sum()
-      numInstances * (math.log(deviance / numInstances * 2.0 * math.Pi) + 1.0) + 2.0 - wt
+    override def aic(predictions: RDD[(Double, Double, Double)],


Likewise there's not a lot of value in pushing 4 separate implementations into one method and using if-else. Just override this method.

srowen · 2016-12-22T12:13:04Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

+  private[regression] class TweedieFamily(private val variancePower: Double)
+    extends Family{
+
+    val name: String = variancePower match {


Why remove the name and switch like this? you can instead adjust this so that subclasses override a name method.

actuaryzhang · 2016-12-22T18:21:35Z

@srowen Thanks for the comments. Makes lots of sense to move the switch to subclass. I did not know one could override a val.

In the new commit, I have moved the defaultLink and aic to subclasses. I also added back name in the constructor. The other methods initialize, variance, deviance and project are common and thus implemented in the TweedieFamily parent class. Hope this makes sense.

@yanboliang

yanboliang

@actuaryzhang Please see my inline comments. Thanks.

yanboliang · 2016-12-23T14:52:27Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

+  final val variancePower: Param[Double] = new Param(this, "variancePower",
+    "The power in the variance function of the Tweedie distribution which characterizes " +
+    "the relationship between the variance and mean of the distribution. " +
+    "Used for the Tweedie family. Supported value: 0 and [1, Inf).",


Used only for should be more clear?

yanboliang · 2016-12-23T14:59:41Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

 * Valid link functions for each family is listed below. The first link function of each family
 * is the default one.
 *  - "gaussian" : "identity", "log", "inverse"
 *  - "binomial" : "logit", "probit", "cloglog"
 *  - "poisson"  : "log", "identity", "sqrt"
 *  - "gamma"    : "inverse", "identity", "log"
+ *  - "tweedie"  : "identity", "log"


- "tweedie" : "log", "identity", see L155: the first link function of each family is the default one.

yanboliang · 2016-12-23T15:03:38Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

-        case Binomial.name => Binomial
-        case Poisson.name => Poisson
-        case Gamma.name => Gamma
+    def fromModel(model: GeneralizedLinearRegressionBase): Family = {


Rename to fromParams, we extract family and variancePower from the Params which is the superclass of GLR estimator and model. And actually we use this function for both estimator(L279) and model(L974).

yanboliang · 2016-12-23T15:16:29Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

+    * Tweedie exponential family distribution.
+    * This includes the special cases of Gaussian, Poisson and Gamma.
+    */
+  private[regression] class TweedieFamily(private val variancePower: Double)


TweedieFamily -> Tweedie, we don't add suffix for other family class/object.

yanboliang · 2016-12-23T15:19:43Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

+    extends Family("tweedie") {
+
+    /*
+      The canonical link is 1 - variancePower. Except for the special cases of Gaussian,


The canonical link is 1 - variancePower, could you clarify this to make it more clear?

yanboliang · 2016-12-23T15:29:23Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

+        require(y >= 0.0, s"The response variable of the specified distribution " +
+          s"should be non-negative, but got $y")
+      } else if (variancePower >= 2.0) {
+        require(y > 0.0, s"The response variable of the specified distribution " +


yanboliang · 2016-12-23T15:36:30Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

+   * Gaussian exponential family distribution.
+   * The default link for the Gaussian family is the identity link.
+   */
+  private[regression] object Gaussian extends TweedieFamily(0.0) {


We should keep the concrete implementation of variance, deviance and aic for Gaussian, Poisson and Gamma, the main reasons are:

These functions were called very frequently, the concrete implementation in subclasses should be more efficient.

It's helpful to locate errors or bugs.

yanboliang · 2016-12-23T15:50:19Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

-    Binomial -> Logit, Binomial -> Probit, Binomial -> CLogLog,
-    Poisson -> Log, Poisson -> Identity, Poisson -> Sqrt,
-    Gamma -> Inverse, Gamma -> Identity, Gamma -> Log
+    "gaussian" -> Identity, "gaussian" -> Log, "gaussian" -> Inverse,


Yeah, you are right. It needs an extra global object to avoid error-prone which may a little expensive. I'm ok with using string.

yanboliang · 2016-12-23T15:51:35Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala


  /** Set of link names that GeneralizedLinearRegression supports. */
  private[regression] lazy val supportedLinkNames = supportedFamilyAndLinkPairs.map(_._2.name)

  private[regression] val epsilon: Double = 1E-16

+  /** Constant used in initialization and deviance to avoid numerical issues. */
+  private[regression] val delta: Double = 0.1


I'm OK to put it here.

yanboliang · 2016-12-23T15:54:34Z

mllib/src/test/scala/org/apache/spark/ml/regression/GeneralizedLinearRegressionSuite.scala

@@ -578,6 +578,100 @@ class GeneralizedLinearRegressionSuite
    }
  }

+  test("generalized linear regression: tweedie family against glm") {
+    /*
+        R code:


library(statmod) which can help users to reproduce this test case.

actuaryzhang · 2016-12-23T19:14:27Z

@yanboliang Thanks much for the detailed comments. I have addressed all of them in the new commits. Please take another look.
@srowen

actuaryzhang · 2016-12-27T05:37:56Z

@srowen @yanboliang
Any additional issues regarding this PR?

srowen

This is down to small items now, I think it's pretty good.

srowen · 2016-12-27T21:14:14Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

     *
-     * @param name family name: "gaussian", "binomial", "poisson" or "gamma".
+     * @param params a GenerealizedLinearRegressionBase object


typo in GenerealizedLinearRegressionBase; this type of doc doesn't do anything though because the type is already documented. It should say something non-trivial

srowen · 2016-12-27T21:21:18Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala


  /** Set of link names that GeneralizedLinearRegression supports. */
  private[regression] lazy val supportedLinkNames = supportedFamilyAndLinkPairs.map(_._2.name)

  private[regression] val epsilon: Double = 1E-16

+  /** Constant used in initialization and deviance to avoid numerical issues. */
+  private[regression] val delta: Double = 0.1


Why not a companion object for TweedieFamily though? that just seems easy and more correct

actuaryzhang · 2016-12-27T22:20:15Z

@srowen Made a new commit according to your suggestion. Everything looking good now?
@yanboliang

yanboliang · 2016-12-28T15:10:41Z

@actuaryzhang I'm on travel these days, and will make another pass in a few days. Thanks.

actuaryzhang · 2017-01-03T18:01:35Z

@yanboliang Did you get a chance to take another look at this? Thanks.

actuaryzhang · 2017-01-18T00:53:16Z

@yanboliang Finally, the test is done. Is there anything else needed for this PR, other than the pending fresh test?

actuaryzhang · 2017-01-19T00:55:56Z

@srowen @yanboliang @felixcheung @jkbradley Could you help kick off the new test please? Thanks.

MLnick · 2017-01-19T07:48:07Z

jenkins add to whitelist

MLnick · 2017-01-19T07:48:17Z

jenkins test this please

SparkQA · 2017-01-19T07:52:45Z

Test build #71643 has started for PR 16344 at commit 83deee3.

actuaryzhang · 2017-01-19T17:34:19Z

Could anybody help me understand what's causing this test to fail? I see several other ML PR failing as well, with the same error message like below:

Error instrumenting class:org.apache.spark.deploy.mesos.ui.MesosClusterUI
Error instrumenting class:org.apache.spark.mllib.regression.IsotonicRegressionModel$SaveLoadV1_0$

Was this related to the recent commit on instrumentation #15671? How do I fix this? Thanks!
@zhengruifeng @yanboliang @jkbradley

yanboliang · 2017-01-20T00:44:37Z

Jenkins, retest this please.

yanboliang · 2017-01-20T00:53:00Z

@actuaryzhang This test failure is caused by Jenkins was not stable, you just need to retest if you encounter similar issue.

SparkQA · 2017-01-20T01:47:27Z

Test build #71686 has finished for PR 16344 at commit 83deee3.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

yanboliang

Only some comments about documents, otherwise, looks good to me. Thanks!

yanboliang · 2017-01-22T09:46:22Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

-      require(supportedFamilyAndLinkPairs.contains(
-        Family.fromName($(family)) -> Link.fromName($(link))), "Generalized Linear Regression " +
-        s"with ${$(family)} family does not support ${$(link)} link function.")
+    if ($(family) == "tweedie") {


$(family).toLowerCase == "tweedie", see #16516, change here and other places.

yanboliang · 2017-01-22T09:59:12Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

-        case Gamma.name => Gamma
+    def fromParams(params: GeneralizedLinearRegressionBase): Family = {
+      params.getFamily match {
+        case "gaussian" => Gaussian


Revert to Gaussian.name here and bellow, which is less error-prone.

yanboliang · 2017-01-22T10:17:14Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

+     */
+    def apply(params: GeneralizedLinearRegressionBase): FamilyAndLink = {
+      val familyObj = Family.fromParams(params)
+      val linkObj = if ((params.getFamily != "tweedie" && params.isDefined(params.link)) ||


isSet is more accurate than isDefined at here, since there is always no default values for both of them. I knew the original code used isDefined, but it's better we can correct them.

yanboliang · 2017-01-22T10:34:06Z

mllib/src/test/scala/org/apache/spark/ml/regression/GeneralizedLinearRegressionSuite.scala

+
+    var idx = 0
+    for (fitIntercept <- Seq(false, true); linkPower <- Seq(0.0, 1.0, -1.0)) {
+      for (variancePower <- Seq(1.6, 2.5)) {


Nit:

for (fitIntercept <- Seq(false, true); linkPower <- Seq(0.0, 1.0, -1.0); variancePower <- Seq(1.6, 2.5)) {

yanboliang · 2017-01-22T10:54:52Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala


  /** @group getParam */
  @Since("2.0.0")
  def getFamily: String = $(family)

  /**
+   * Param for the power in the variance function of the Tweedie distribution which provides
+   * the relationship between the variance and mean of the distribution.
+   * Used only for the Tweedie family.


Nit: Only applicable for "tweedie" family. should be better?

yanboliang · 2017-01-22T16:06:07Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

-     * Gets the [[Family]] object from its name.
+     * Gets the [[Family]] object based on family and variancePower.
+     * 1) retrieve object based on family name
+     * 2) if family name is tweedie, retrieve object based on variancePower


Should the following document be better?

Gets the Family object based on param family and variancePower. If param family was set with "gaussian", "binomial", "poisson" or "gamma", return the corresponding object directly; otherwise, construct a Tweedie object according to variancePower.

yanboliang · 2017-01-22T16:20:15Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

@@ -620,25 +779,67 @@ object GeneralizedLinearRegression extends DefaultParamsReadable[GeneralizedLine
  private[regression] object Link {

    /**
-     * Gets the [[Link]] object from its name.
+     * Gets the [[Link]] object based on link or linkPower.


Gets the Link object based on param family, link and linkPower. If param family was set with "tweedie", return or construct link function object according to linkPower; otherwise, return link function object according to link.

yanboliang · 2017-01-22T16:22:20Z

mllib/src/main/scala/org/apache/spark/ml/regression/GeneralizedLinearRegression.scala

     *
-     * @param name link name: "identity", "logit", "log",
-     *             "inverse", "probit", "cloglog" or "sqrt".
+     * @param params the parameter map containing link and link power


link and link power -> family, link and linkPower

yanboliang · 2017-01-22T16:29:49Z

mllib/src/test/scala/org/apache/spark/ml/regression/GeneralizedLinearRegressionSuite.scala

+        1.0, 3.0, 2.0, 1.0,
+        0.0, 4.0, 3.0, 3.0), 4, 4, byrow = TRUE))
+
+      f <- glm(V1 ~ -1 + V3 + V4, data = df, weights = V2,


Change f to model, and add summary(model) at next line. It's helpful for users to reproduce the result in R.

yanboliang · 2017-01-22T16:36:55Z

mllib/src/test/scala/org/apache/spark/ml/regression/GeneralizedLinearRegressionSuite.scala

+
+      Number of Fisher Scoring iterations: 11
+
+      residuals(model, type="pearson")


Typos here? I guess you paste irrelevant results for the following residuals, they should be consistent with L1279-L1281.

SparkQA · 2017-01-23T02:53:04Z

Test build #71817 has finished for PR 16344 at commit 995e88f.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

actuaryzhang · 2017-01-23T04:02:21Z

@yanboliang Thanks so much for your detailed review. Your suggestions make lots of sense and I have included all of them in the new commit. Let me know if there is any other change needed.

SparkQA · 2017-01-23T04:59:08Z

Test build #71823 has finished for PR 16344 at commit 54da2cb.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

yanboliang · 2017-01-25T15:33:01Z

This looks good to me. cc @jkbradley @srowen @sethah for another pass if you have time. Thanks.

yanboliang · 2017-01-27T07:02:49Z

Merged into master. If there are comments from others, we can address them in follow-up work. Thanks.

## What changes were proposed in this pull request? I propose to add the full Tweedie family into the GeneralizedLinearRegression model. The Tweedie family is characterized by a power variance function. Currently supported distributions such as Gaussian, Poisson and Gamma families are a special case of the Tweedie https://en.wikipedia.org/wiki/Tweedie_distribution. yanboliang srowen sethah Author: actuaryzhang <[email protected]> Author: Wayne Zhang <[email protected]> Closes apache#16344 from actuaryzhang/tweedie.

…on.linkPower Add Scaladoc for GeneralizedLinearRegression.linkPower default value Follow-up to apache#16344 Author: Joseph K. Bradley <[email protected]> Closes apache#17069 from jkbradley/tweedie-comment.

Update GLM documentation to include the Tweedie distribution. apache#16344 jkbradley yanboliang Author: actuaryzhang <[email protected]> Closes apache#17103 from actuaryzhang/doc.

## What changes were proposed in this pull request? Port Tweedie GLM #16344 to SparkR felixcheung yanboliang ## How was this patch tested? new test in SparkR Author: actuaryzhang <[email protected]> Closes #16729 from actuaryzhang/sparkRTweedie.

actuaryzhang added 3 commits December 15, 2016 16:50

Add Tweedie family to GLM

952887e

Fix calculation in dev resid; Add test for different var power

4f184ec

Merge test into GLR

7fe3910

srowen requested changes Dec 20, 2016

View reviewed changes

srowen reviewed Dec 20, 2016

View reviewed changes

Use Tweedie class instead of global object Tweedie; change variancePo…

bfcc4fb

…wer to varPower

yanboliang reviewed Dec 21, 2016

View reviewed changes

srowen requested changes Dec 21, 2016

View reviewed changes

actuaryzhang added 3 commits December 21, 2016 15:42

Allow Family to use GLRBase object directly

a8feea7

Add TweedieFamily and implement specific distn within Tweedie

233e2d3

Clean up doc

17c5581

srowen requested changes Dec 22, 2016

View reviewed changes

actuaryzhang added 2 commits December 22, 2016 09:52

Move defaultLink and name to subclass of TweedieFamily

0b41825

Change style for AIC

6e8e607

yanboliang reviewed Dec 23, 2016

View reviewed changes

actuaryzhang added 2 commits December 23, 2016 11:10

Rename Family methods and restore methods for tweedie subclasses

8d7d34e

Update test

6da7e30

Clean up doc

9a71e89

srowen reviewed Dec 27, 2016

View reviewed changes

Put delta in Tweedie companion object

f461c09

Clean up doc

a839c46

yanboliang reviewed Jan 22, 2017

View reviewed changes

actuaryzhang added 2 commits January 22, 2017 17:51

Merge branch 'master' into tweedie

995e88f

update docs

54da2cb

actuaryzhang mentioned this pull request Jan 23, 2017

[SPARK-19155][ML] Make family case insensitive in GLM #16675

Closed

asfgit closed this in 4172ff8 Jan 27, 2017

actuaryzhang mentioned this pull request Jan 28, 2017

[SPARK-19391][SparkR][ML] Tweedie GLM API for SparkR #16729

Closed

actuaryzhang deleted the tweedie branch February 1, 2017 00:28

actuaryzhang mentioned this pull request Feb 14, 2017

[SPARK-18710][ML] Add offset in GLM #16699

Closed

jkbradley mentioned this pull request Feb 26, 2017

[MINOR][ML][DOC] Document default value for GeneralizedLinearRegression.linkPower #17069

Closed

actuaryzhang mentioned this pull request Feb 28, 2017

[Minor][Doc] Update GLM doc to include tweedie distribution #17103

Closed


		override def initialize(y: Double, weight: Double): Double = y
		override def variance(mu: Double): Double = {


		Number of Fisher Scoring iterations: 11

		residuals(model, type="pearson")

[SPARK-18929][ML] Add Tweedie distribution in GLM #16344

[SPARK-18929][ML] Add Tweedie distribution in GLM #16344

Conversation

actuaryzhang commented Dec 19, 2016 • edited Loading

What changes were proposed in this pull request?

yanboliang commented Dec 20, 2016

srowen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

actuaryzhang Dec 20, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srowen Dec 21, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

actuaryzhang commented Dec 20, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

actuaryzhang Dec 21, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srowen Dec 21, 2016 • edited Loading

Choose a reason for hiding this comment

actuaryzhang commented Dec 22, 2016

srowen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

actuaryzhang commented Dec 22, 2016 • edited Loading

yanboliang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

actuaryzhang commented Dec 23, 2016

actuaryzhang commented Dec 27, 2016

srowen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

actuaryzhang commented Dec 27, 2016

yanboliang commented Dec 28, 2016

actuaryzhang commented Jan 3, 2017

actuaryzhang commented Jan 18, 2017 • edited Loading

actuaryzhang commented Jan 19, 2017

MLnick commented Jan 19, 2017

MLnick commented Jan 19, 2017

SparkQA commented Jan 19, 2017

actuaryzhang commented Jan 19, 2017 • edited Loading

yanboliang commented Jan 20, 2017

yanboliang commented Jan 20, 2017

SparkQA commented Jan 20, 2017

yanboliang left a comment

actuaryzhang commented Dec 19, 2016 •

edited

Loading

actuaryzhang Dec 20, 2016 •

edited

Loading

srowen Dec 21, 2016 •

edited

Loading

actuaryzhang Dec 21, 2016 •

edited

Loading

srowen Dec 21, 2016 •

edited

Loading

actuaryzhang commented Dec 22, 2016 •

edited

Loading

actuaryzhang commented Jan 18, 2017 •

edited

Loading

actuaryzhang commented Jan 19, 2017 •

edited

Loading