[SPARK-14682][ML] Provide evaluateEachIteration method or equivalent for spark.ml GBTs #21097
Conversation
|
Test build #89499 has finished for PR 21097 at commit
|
|
Test build #89500 has finished for PR 21097 at commit
|
| @@ -365,6 +365,20 @@ class GBTClassifierSuite extends MLTest with DefaultReadWriteTest { | |||
| assert(mostImportantFeature !== mostIF) | |||
| } | |||
|
|
|||
| test("model evaluateEachIteration") { | |||
| for (lossType <- Seq("logistic")) { | |||
There was a problem hiding this comment.
there is only one lossType. for is not necessary.
There was a problem hiding this comment.
Yes. But I think it can fit for future, if we add more loss type for GBT classifier.
| .setLossType(lossType) | ||
| val model = gbt.fit(trainData.toDF) | ||
| val eval1 = model.evaluateEachIteration(validationData.toDF) | ||
| val eval2 = GradientBoostedTrees.evaluateEachIteration(validationData, |
There was a problem hiding this comment.
This is testing the spark.ml implementation against itself. I was about to recommend using the old spark.mllib implementation as a reference. However, the old implementation is not tested at all. Would you be able to test against a standard implementation in R or scikit-learn (following the patterns used elsewhere in MLlib)?
There was a problem hiding this comment.
I search scikit-learn doc, there seems no similar method like evaluateEachIteration, we can only use staged_predict in sklearn.ensemble.GradientBoostingRegressor and then use metric functions to evaluate them. And I doubt the implementation differ slightly in other library will be troublesome. In R package I also do not find this method.
Now I update the unit test, to just compare with hardcoded result.
| * @param dataset Dataset for validation. | ||
| */ | ||
| @Since("2.4.0") | ||
| def evaluateEachIteration(dataset: Dataset[_]): Array[Double] = { |
There was a problem hiding this comment.
Do we want to support evaluation on other losses, as in the old API? It might be nice to be able to without having to modify the Model's loss Param value.
|
Test build #90188 has finished for PR 21097 at commit
|
There was a problem hiding this comment.
For unit tests, what about this?
- Used a fixed random seed.
- Run for maxIter = 3
- Create models with 1 and 2 trees by manually getting the trees and constructing new GBT models.
- Check to make sure the loss for a model with 1 tree matches the first value returned by evaluateEachIteration for the other 2 models.
- Check to make sure the loss for a model with 2 trees matches the second value returned by evaluateEachIteration for the model with 3 trees.
| /** | ||
| * Method to compute error or loss for every iteration of gradient boosting. | ||
| * | ||
| * @param dataset Dataset for validation. |
There was a problem hiding this comment.
Add doc for "loss" arg, including what the options are
|
Test build #90351 has finished for PR 21097 at commit
|
| val model2 = new GBTClassificationModel("gbt-cls-model-test2", | ||
| model3.trees.take(2), model3.treeWeights.take(2), model3.numFeatures, model3.numClasses) | ||
|
|
||
| for (evalLossType <- GBTClassifier.supportedLossTypes) { |
There was a problem hiding this comment.
evalLossType is not used, so I'd remove this loop.
|
Test build #90404 has finished for PR 21097 at commit
|
|
LGTM |
What changes were proposed in this pull request?
Provide evaluateEachIteration method or equivalent for spark.ml GBTs.
How was this patch tested?
UT.
Please review http://spark.apache.org/contributing.html before opening a pull request.