[XGBoost4J-Spark] bestIteration and bestScore for early stopping #7095

naveenkb · 2021-07-08T15:15:17Z

As per discussions in #6893, adding bestIteration and bestScore attributes.

sample code snippet to access the attributes

.
.
.
val xgbClassificationModel = xgbClassifier.fit(train)

val bestScore = xgbClassificationModel.nativeBooster.getAttr("bestScore")
val bestIteration = xgbClassificationModel.nativeBooster.getAttr("bestIteration")

trivialfis

Thanks for the PR! I'm not familiar with the jvm package, just providing suggestions. Also is there a way we can have an unittest?

cc @wbo4958

trivialfis · 2021-07-08T17:19:09Z

jvm-packages/xgboost4j/src/main/java/ml/dmlc/xgboost4j/java/XGBoost.java

@@ -242,11 +242,15 @@ public static Booster trainAndSaveCheckpoint(
          if (score > bestScore) {
            bestScore = score;
            bestIteration = iter;
+            booster.setAttr("bestIteration", String.valueOf(bestIteration));


It might be better to use best_iteration to align with other language bindings.

So, the best iteration and best score are decided by all other language bindings? @trivialfis

Yeah, we'd better to keep the same naming for all language bindings.

@trivialfis - Using camel case is the norm in Java so I went ahead with bestIteration and bestScore. This is in line with other naming convention used in Booster.java & XGBoost.java code files. Let me know if you still would like to change it to best_iteration.

So, the best iteration and best score are decided by all other language bindings? @trivialfis

Yes.

@trivialfis - Using camel case is the norm in Java so I went ahead with bestIteration and bestScore

For java attributes that make sense. But for the string stored inside XGBoost, please use snake case.

booster.setAttr("best_iteration", String.valueOf(bestIteration));

Notice the difference between string and java symbol. This way the model is more portable.

Makes sense. Updated with snake case

wbo4958

Hi naveenkb, Could we add a unit test for this PR and Better update the doc for this, @trivialfis is there any doc to describe the best iteration/score things?

wbo4958 · 2021-07-08T23:52:41Z

jvm-packages/xgboost4j/src/test/java/ml/dmlc/xgboost4j/java/BoosterImplTest.java

@@ -643,7 +643,7 @@ public void testSetAndGetAttrs() throws XGBoostError {
    }});

    Map<String, String> attr = booster.getAttrs();
-    TestCase.assertEquals(attr.size(), 4);
+    TestCase.assertEquals(attr.size(), 6);


Could you add some comments why the attr.size() is equal to 6?

@wbo4958 maybe because 2 attributes got added?

Yeah, I know the extra 2 attrs, but others may be so confused. Better to add some comments here

@wbo4958 Yes as attr.size() is 6 because 2 additional attributes are added. The test case is checking setter and getter method and also total number of attributes in the booster object.
I will update the doc to explain best iteration/score.

Any suggestion on adding unit test for these attribute ?

checking if the booster is containing best_iteration and best_score?

wbo4958 · 2021-07-08T23:53:36Z

jvm-packages/xgboost4j/src/main/java/ml/dmlc/xgboost4j/java/XGBoost.java

@@ -242,11 +242,15 @@ public static Booster trainAndSaveCheckpoint(
          if (score > bestScore) {
            bestScore = score;
            bestIteration = iter;
+            booster.setAttr("bestIteration", String.valueOf(bestIteration));


So, the best iteration and best score are decided by all other language bindings? @trivialfis

trivialfis · 2021-07-14T17:59:44Z

Please ignore the failure for now. Something on our CI updated we are still trying to figure out what's broken.

trivialfis

Looks good to me, please rebase onto the master branch to get the CI passed. Thanks for the feature!

codecov-commenter · 2021-07-17T15:39:47Z

Codecov Report

Merging #7095 (e07af5d) into master (d7c1449) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #7095   +/-   ##
=======================================
  Coverage   81.60%   81.60%           
=======================================
  Files          13       13           
  Lines        3903     3903           
=======================================
  Hits         3185     3185           
  Misses        718      718

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d7c1449...e07af5d. Read the comment docs.

trivialfis reviewed Jul 8, 2021

View reviewed changes

wbo4958 reviewed Jul 9, 2021

View reviewed changes

trivialfis approved these changes Jul 16, 2021

View reviewed changes

naveenkb added 3 commits July 17, 2021 19:54

Expose bestIteration and bestScore attributes

983f2d1

remove redundant wrt to model slicer

b115303

Change bestIteration and bestScore attribute to snake case

e07af5d

naveenkb force-pushed the bestIteration branch from 937ce4d to e07af5d Compare July 17, 2021 15:00

wbo4958 approved these changes Jul 19, 2021

View reviewed changes

trivialfis merged commit 9f7f8b9 into dmlc:master Jul 19, 2021

naveenkb deleted the bestIteration branch July 19, 2021 12:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[XGBoost4J-Spark] bestIteration and bestScore for early stopping #7095

[XGBoost4J-Spark] bestIteration and bestScore for early stopping #7095

naveenkb commented Jul 8, 2021

trivialfis left a comment •

edited

Loading

trivialfis Jul 8, 2021

wbo4958 Jul 8, 2021

wbo4958 Jul 9, 2021

naveenkb Jul 13, 2021

trivialfis Jul 13, 2021 •

edited

Loading

naveenkb Jul 14, 2021

wbo4958 left a comment

wbo4958 Jul 8, 2021

candalfigomoro Jul 9, 2021

wbo4958 Jul 9, 2021

naveenkb Jul 13, 2021

wbo4958 Jul 14, 2021

wbo4958 Jul 8, 2021

trivialfis commented Jul 14, 2021 •

edited

Loading

trivialfis left a comment

codecov-commenter commented Jul 17, 2021 •

edited

Loading

[XGBoost4J-Spark] bestIteration and bestScore for early stopping #7095

[XGBoost4J-Spark] bestIteration and bestScore for early stopping #7095

Conversation

naveenkb commented Jul 8, 2021

trivialfis left a comment • 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

trivialfis Jul 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wbo4958 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

trivialfis commented Jul 14, 2021 • edited Loading

trivialfis left a comment

Choose a reason for hiding this comment

codecov-commenter commented Jul 17, 2021 • edited Loading

Codecov Report

trivialfis left a comment •

edited

Loading

trivialfis Jul 13, 2021 •

edited

Loading

trivialfis commented Jul 14, 2021 •

edited

Loading

codecov-commenter commented Jul 17, 2021 •

edited

Loading