-
Notifications
You must be signed in to change notification settings - Fork 2k
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
AutoML with XGBoost Only Fails on Learning Rate Search #7265
Comments
Veronika Maurerová commented: This is a new validation of XGBoost aliases, where we were allowed to use both parameters, and one rewrites the other in the past. Here is the Jira where I fixed it and implemented the check that both parameters cant be set both. [https://h2oai.atlassian.net/browse/PUBDEV-8266|https://h2oai.atlassian.net/browse/PUBDEV-8266|smart-link] So [~accountid:5b153fb1b0d76456f36daced] , first check that in AutoML you are not setting both parameters somewhere. |
Marek Novotny commented: [~accountid:5bd237b8dd3cc64b77e71676] Has anything in the case when only the original parameter (ntrees) without an alias (n_estimators) is sent to the h2o backend? |
Veronika Maurerová commented: The h2o backend should not set both parameters. I am not sure if in AutoML it somewhere happens. If a user sets only one parameter, the h2o backend should use only this one. In this case, it says: {noformat} _ntrees: ntrees and its alias n_estimators are both set to different value than default value. {noformat} So it means both parameters were set to different value and the h2o backend is not sure which one to use. |
Veronika Maurerová commented: This can happen for example if the old parameter object is used again and only one parameter is changed to a different value. For example parameter object has ntrees = 10 & n_estimators = 10 which is ok. If you then change only ntree to 6, you sent parameters object with ntrees = 6 & n_estimators = 10, which causes the error. |
Sebastien Poirier commented: [~accountid:5bd237b8dd3cc64b77e71676] I think it’s a very bad idea to raise an error when both are set, especially when {{n_estimators}} has been deprecated for years, and in some (admittedly rare) cases, we’re now forcing users to set this deprecated param, preventing us from deleting it completely in the future without breaking code (it’s equivalent to resurrecting a dead param). Here is the AutoML use-case and many users could do the same on their side:
If you want to keep {{n_estimators}} alias (and other aliases), I think it should appear only in the schema and be deleted everywhere else and the synchronization logic should be done there and only there ({{n_estimators}} schema property actually setting the value to {{n_trees}} param). |
Veronika Maurerová commented: Resolution from the private conversation:
|
JIRA Issue Details Jira Issue: PUBDEV-8394 |
Linked PRs from JIRA |
Stack Trace:
The text was updated successfully, but these errors were encountered: