-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
[Breaking] Change default evaluation metric for binary:logitraw objective to logloss #6647
Conversation
<< "' to '" << after << "'. Explicitly set eval_metric if you'd like to " | ||
<< "restore the old behavior."; | ||
const std::string& after, const std::string& version) { | ||
LOG(WARNING) << "Starting in XGBoost " << version << ", the default evaluation metric " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note. We should plan to remove this warning soon. Probably right after 1.4.0 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The warning got extra verbose when using distributed training. All workers are printing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the warning go to logging
or stdout?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hcho3 stdout
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the _log_calback
function in xgboost/core.py
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. We could modify _log_callback
to use the logging
module. Alternatively, we could simply remove the warning message here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do both in the future, but it will be another breaking change. To use logging
, one needs to know the verbosity. So _log_callback
should have 2 parameters _log_callback(msg: str, verbosity: int)
to indicate the level of current message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. Let's remove the warning message after 1.4.0 release.
Is there a test for the changed behaviour or a test to adapt because of the change? |
Unlike #6183, not much of the existing code is affected by this change, since |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, too 👍
Closes #6618
cc @lorentzenchr @mayer79