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

clarify DEBUG-level log about tree depth #4126

Merged
merged 2 commits into from
Apr 5, 2021
Merged

clarify DEBUG-level log about tree depth #4126

merged 2 commits into from
Apr 5, 2021

Conversation

jameslamb
Copy link
Collaborator

*TreeLearner::Train() methods have a DEBUG-level log like this:

[LightGBM] [Debug] Trained a tree with leaves = 31 and max_depth = 6
[LightGBM] [Debug] Trained a tree with leaves = 31 and max_depth = 7
[LightGBM] [Debug] Trained a tree with leaves = 31 and max_depth = 8
[LightGBM] [Debug] Trained a tree with leaves = 31 and max_depth = 8
[LightGBM] [Debug] Trained a tree with leaves = 31 and max_depth = 7
[LightGBM] [Debug] Trained a tree with leaves = 31 and max_depth = 9
[LightGBM] [Debug] Trained a tree with leaves = 31 and max_depth = 8
[LightGBM] [Debug] Trained a tree with leaves = 31 and max_depth = 9
[LightGBM] [Debug] Trained a tree with leaves = 31 and max_depth = 7
[LightGBM] [Debug] Trained a tree with leaves = 31 and max_depth = 9

This message is saying "ok, this tree is complete. It has n leaves and depth d". I think it's confusing to refer to the depth of that tree as max_depth, since that is also the name of a LightGBM parameter (https://lightgbm.readthedocs.io/en/latest/Parameters.html#max_depth). When I first saw logs like this, I thought "wait, why is LightGBM changing the max_depth parameter at each training iteration?".

This PR proposes changing that language to "depth" to avoid such confusion.

Linking to #4026 (comment).

@jameslamb jameslamb merged commit 6d825cd into master Apr 5, 2021
@jameslamb jameslamb deleted the fix/depth-log branch April 5, 2021 13:28
@StrikerRUS
Copy link
Collaborator

@jameslamb Please do not merge PRs when we have a pending blocking CI PR because it often results in outdated nightly releases and may confuse users.

@jameslamb
Copy link
Collaborator Author

oh! Sorry, did not consider the implications for nightly builds.

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants