-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
[doc] Warning message says "you didn't set num_leaves" even if I explicitly set num_leaves=31 #2898
Comments
Hello @bfrobin446 , What would you like to see instead? "Accuracy may be bad as num_leaves < 2^max_depth, with num_leaves=x." ? |
Maybe something like "The default num_leaves setting of 31 may be too low for best accuracy when 2^max_depth > num_leaves. Consider increasing num_leaves or reducing max_depth." |
I'll leave that to @StrikerRUS, what's your input on this? |
I agree that the current warning message is far from perfect. Contribution is more than welcome! Feel free to propose a PR. |
Will do, thanks :)
@bfrobin446, do you want me to do it or since you were so precise with the description, do you want to get your hands dirty and do it yourself? I'm fine either way, just let me know what you prefer ;)
…On Fri, 20 Mar 2020, 20:59 Nikita Titov, ***@***.***> wrote:
I agree that the current warning message is far from perfect. Contribution
is more than welcome! Feel free to propose a PR.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#2898 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA6K4MI2NPSXYWA5F4LHYZLRIPKLRANCNFSM4LF6LEIQ>
.
|
Agree with @bfrobin446 . This happens when I'm refering to Chinese documentations, where said we should let
what are you really suggesting? Should be 2^max_depth > num_leaves or 2^max_depth < num_leaves?
Agreements would likely be reached. |
I can submit a PR this or next week as bfrobin446 didn't reply. |
#3537) * Fix #2898: Clearer warning message (2^max_depth > num_leaves). * Update src/io/config.cpp Co-authored-by: Nikita Titov <[email protected]> * Update src/io/config.cpp Co-authored-by: Nikita Titov <[email protected]> Co-authored-by: Nikita Titov <[email protected]>
Sorry for revisiting this, but this message is shown when verbosity == -1, isn't that a bug? It's certainly not fatal. |
But why in https://lightgbm.readthedocs.io/en/latest/Parameters-Tuning.html, the doc tell us to set |
@guolinke or @shiyu1994 could you comment on @Wang-Yu-Qing 's question? I agree that the warning message and documentation seem to be making contradictory recommendations, and I'm not qualified to say which is the correct one. |
The warning message is actually saying: you forget to set The document is emphasizing that My proposal is to keep the document unchanged. But in |
Ok thanks for the explanation @shiyu1994 ! Given that, I think we should remove this warning entirely, and rely on documentation to help users choose good values (and, in practice, good ranges to explore during tuning). Because it seems to me that there are legitimate reasons to want either |
Environment info
Operating System: Ubuntu 18.04.3 LTS
CPU/GPU model: Not sure (running in AWS EC2)
C++/Python/R version: GCC 7.5/Python 3.7.5
LightGBM version or commit hash: 6b6709
Error message
[LightGBM] [Warning] Accuracy may be bad since you didn't set num_leaves and 2^max_depth > num_leaves
Steps to reproduce
Fit any model with num_leaves = 31 and max_depth >= 5
At line 307 of src/io/config.cpp, the check for whether to show the warning can't tell whether num_leaves is 31 because that's the default or because 31 was explicitly passed. The text "you didn't set num_leaves" in the message is confusing if I did explicitly set num_leaves to 31.
The text was updated successfully, but these errors were encountered: