Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[jvm-packages] Automatically set maximize_evaluation_metrics if not explicitly given in XGBoost4J-Spark #4446

Merged
merged 4 commits into from
May 9, 2019

Conversation

shishaochen
Copy link
Contributor

@shishaochen shishaochen commented May 8, 2019

This pull request is aimed at resolving "[jvm-packages] false should be the default value of maximize_evaluation_metrics"(#4422).
The metric names to maximize are aligned with xgboost.callback.early_stop in the Python package.
To avoid duplicating codes in both XGBoostClassifier and XGBoostRegressor, parameter maximize_evaluation_metrics is set in XGBoost.scala when not provided by user.

closes #4422

@hcho3 hcho3 changed the title [XGBoost4j-Spark] Automatically set maximize_evaluation_metrics if not explicitly given. [jvm-packages] Automatically set maximize_evaluation_metrics if not explicitly given in XGBoost4J-Spark May 8, 2019
@CodingCat
Copy link
Member

thanks, would you please also update https://xgboost.readthedocs.io/en/latest/jvm/xgboost4j_spark_tutorial.html#early-stopping accordingly?

@shishaochen
Copy link
Contributor Author

@CodingCat Document is updated in new commits.
When custom_eval is set, maximize_evaluation_metrics is still required.

@CodingCat
Copy link
Member

@hcho3 jenkins says [2019-05-09T03:31:04.298Z] E: You don't have enough free space in /var/cache/apt/archives/.?

@hcho3
Copy link
Collaborator

hcho3 commented May 9, 2019

@CodingCat Looks like we need to expand the volume for the slave node

@shishaochen
Copy link
Contributor Author

It seems that 2 failures of checks are not caused by my code change but the disk volume.
Coming back to this PR, is there anything else need I do? @CodingCat

@hcho3
Copy link
Collaborator

hcho3 commented May 9, 2019

I’m looking at the disk space issue.

@hcho3
Copy link
Collaborator

hcho3 commented May 9, 2019

@CodingCat @rongou I went ahead and increased the disk space.

@CodingCat CodingCat merged commit 18e4fc3 into dmlc:master May 9, 2019
@@ -160,7 +160,7 @@ object XGBoost extends Serializable {
.map(_.toString.toInt).getOrElse(0)
val overridedParams = if (numEarlyStoppingRounds > 0 &&
!params.contains("maximize_evaluation_metrics")) {
if params.contains("custom_eval") {
if (params.contains("custom_eval")) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when num_early_stopping_rounds is non-zero, eval_metric is a built-in metric, the custom_eval is still in params, just the value is null, so it will still throw an exception.
So the if condition should be if (params.contains("custom_eval") && params("custom_eval") != null) or remove custom_eval from params.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm....I think I missed some part of this PR

The reality is early stopping doesn’t work with custom metrics

@lock lock bot locked as resolved and limited conversation to collaborators Aug 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[jvm-packages] false should be the default value of maximize_evaluation_metrics
4 participants