[SPARK-9073][ML] spark.ml Models copy() should call setParent when there is a parent

[Lewuathe]

Copied ML models must have the same parent of original ones

[SparkQA]

Test build #37502 has finished for PR 7447 at commit 6a5b495.

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

[SparkQA]

Test build #38842 has finished for PR 7447 at commit f091a6b.

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

[jkbradley]

@Lewuathe Sorry for the delay, but this would actually be important to get into Spark 1.5. Would you have time to fix the merge conflicts, and then I can make a final pass? Thank you!

[Lewuathe]

@jkbradley I merged master into this branch. Could you check it? Thank you!

[SparkQA]

Test build #39661 has finished for PR 7447 at commit 5316908.

• This patch fails SparkR unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[jkbradley]

fix indentation

[jkbradley]

organize imports (Please check below for other files too.)

[jkbradley]

@Lewuathe Thanks for the update. Looking at this again, I've become more convinced that the check should piggy-back on other tests. For most models, it fits naturally with the standard ParamsSuite.checkParams check. For a few models, it's pretty expensive to add a unit test which requires fitting a new model. Could you please put the check into existing unit tests?

[SparkQA]

Test build #40081 has finished for PR 7447 at commit 1e0b610.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • sealed trait LogisticRegressionTrainingSummary extends LogisticRegressionSummary
  • sealed trait LogisticRegressionSummary extends Serializable
  • class IsotonicRegression(override val uid: String) extends Estimator[IsotonicRegressionModel]
  • final class SpecificMutableRow(val values: Array[MutableValue])
  • final class SpecificRow extends $
  • trait BaseGenericInternalRow extends InternalRow
  • class GenericMutableRow(values: Array[Any]) extends MutableRow with BaseGenericInternalRow

[SparkQA]

Test build #40106 has finished for PR 7447 at commit 4c60f8d.

• This patch fails SparkR unit tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • sealed trait LogisticRegressionTrainingSummary extends LogisticRegressionSummary
  • sealed trait LogisticRegressionSummary extends Serializable
  • class IsotonicRegression(override val uid: String) extends Estimator[IsotonicRegressionModel]
  • final class SpecificMutableRow(val values: Array[MutableValue])
  • final class SpecificRow extends $
  • trait BaseGenericInternalRow extends InternalRow
  • class GenericMutableRow(values: Array[Any]) extends MutableRow with BaseGenericInternalRow
  • case class StringTranslate(srcExpr: Expression, matchingExpr: Expression, replaceExpr: Expression)

[Lewuathe]

@jkbradley Could you check it? We can keep separate test case because some suite such as RandomForestRegressionSuite has no param check case.

[SparkQA]

Test build #40289 has finished for PR 7447 at commit 5d798e9.

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

[jkbradley]

@Lewuathe I realized my suggestion of including the check within the Params check was not a good idea since those checks create models without parents. (The parent check therefore just ensures that null equals null.) Sorry for asking for yet another update, but could you please update the tests to piggy-back off of other tests which create models via fit()? Those tests will create models with actual parents, so that the checkCopy() method will do a reasonable test. Please let me know if this makes sense to you.

Thank you!

[Lewuathe]

@jkbradley That sounds good to me at last. I'll update. Thank you!

[SparkQA]

Test build #40449 has finished for PR 7447 at commit 9445cb7.

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

[jkbradley]

fix indentation

[jkbradley]

This should be in a different test. The model should be produced via fit(), not constructed from scratch, so that the model has a parent.

[SparkQA]

Test build #40625 has finished for PR 7447 at commit 200dd11.

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

[jkbradley]

I should have been clearer: Can you please check both that the uids match and the equality operator?

assert(copied.parent.uid == model.parent.uid)
assert(copied.parent == model.parent)

[jkbradley]

Looks good except that 1 item

[SparkQA]

Test build #40759 has finished for PR 7447 at commit c01e93c.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds the following public classes (experimental):
  • trait Identifiable
  • class VectorUDT extends UserDefinedType[Vector]
  • class KMeans(JavaEstimator, HasFeaturesCol, HasPredictionCol, HasMaxIter, HasTol, HasSeed):

[jkbradley]

LGTM. Merging with master and branch-1.5
Thanks!