[SPARK-13784][ML] Persistence for RandomForestClassifier, RandomForestRegressor

[jkbradley]

What changes were proposed in this pull request?

Main change: Added save/load for RandomForestClassifier, RandomForestRegressor (implementation details below)

Modified numTrees method (deprecation)

• Goal: Use default implementations of unit tests which assume Estimators and Models share the same set of Params.
• What this PR does: Moves method numTrees outside of trait TreeEnsembleModel. Adds it to GBT and RF Models. Deprecates it in RF Models in favor of new method getNumTrees. In Spark 2.1, we can have RF Models include Param numTrees.

Minor items

• Fixes bugs in GBTClassificationModel, GBTRegressionModel fromOld methods where they assign the wrong old UID.

Implementation details

• Split DecisionTreeModelReadWrite.loadTreeNodes into 2 methods in order to reuse some code for ensembles.
• Added EnsembleModelReadWrite object with save/load implementations usable for RFs and GBTs
  • These store all trees' nodes in a single DataFrame, and all trees' metadata in a second DataFrame.
• Split trait RandomForestParams into parts in order to add more Estimator Params to RF models
• Split DefaultParamsWriter.saveMetadata into two methods to allow ensembles to store sub-models' metadata in a single DataFrame. Same for DefaultParamsReader.loadMetadata

How was this patch tested?

Adds standard unit tests for RF save/load

[SparkQA]

Test build #54727 has finished for PR 12118 at commit 607ed4e.

• This patch fails Scala style tests.
• This patch does not merge cleanly.
• This patch adds no public classes.

[jkbradley]

@GayathriMurali @hhbyyh Could you please help review this PR? Thanks!

[SparkQA]

Test build #54730 has finished for PR 12118 at commit cd07aff.

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

[jkbradley]

That seems like a spurious or unrelated failure. I'll retest

[SparkQA]

Test build #54728 has finished for PR 12118 at commit b13b11f.

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

[SparkQA]

Test build #2727 has finished for PR 12118 at commit cd07aff.

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

[GayathriMurali]

Is it alright to use flatMap to combine RDDs? Can we use sparkContext.union instead?

[jkbradley]

This is a single RDD. The flatMap maps every element of the original RDD to multiple elements in a new RDD. It should be fine.

[GayathriMurali]

@jkbradley Thanks for this. This looks great and clarifies a lot of things I was trying to do. I had one minor comment, except that it looks fine to me.

[jkbradley]

@GayathriMurali Thanks for taking a look! @hhbyyh It would be great if you could take a look too since you're familiar with some of this code.

[hhbyyh]

Sorry for the delay. I'll start now.

[hhbyyh]

indent correctly?
Similar for a few lines next.

[hhbyyh]

just some minor comments. LGTM.

[SparkQA]

Test build #54828 has finished for PR 12118 at commit afed67b.

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

[SparkQA]

Test build #54836 has finished for PR 12118 at commit e0306a9.

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

[jkbradley]

@GayathriMurali Thanks for the initial commits, and @hhbyyh thanks for the review! I'll go ahead and merge this with master since you gave a LGTM pending minor changes.