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

[python-package] respect 'verbose' setting when using custom objective function (fixes #6014) #6428

Merged
merged 7 commits into from
Jul 12, 2024

Conversation

jameslamb
Copy link
Collaborator

@jameslamb jameslamb commented Apr 25, 2024

Fixes #6014.

Proposes changing the behavior of Config::SetVerbosity() when it's passed params that don't contain verbose or verbosity. Instead of resetting the log level to INFO in those situations, as of this PR it would not modify the log level at all.

See #6014 (comment) for an explanation of this.

Notes for Reviewers

How I tested this

Using the examples in #6014.

And I've added 2 new unit tests here (one for lgb.train(), one for the scikit-learn interface) testing the behavior. These tests fail on master.

I also ran each of the Python test files one at a time, to be sure they weren't accidentally relying on log level state set by previous tests.

pytest tests/python_package_test/test_basic.py
pytest tests/python_package_test/test_callback.py
# etc., etc.

A note on usability

Prior to this change, from the perspective of the Python package it was sort of a hidden implementation detail that the logging level is a process-global variable.

// a trick to use static variable in header file.
// May be not good, but avoid to use an additional cpp file
static LogLevel &GetLevel() {
static THREAD_LOCAL LogLevel level = LogLevel::Info;
return level;
}

Running lgb.train(dtrain) with all parameters at their defaults would always generate INFO-level logs.

As of this change, passing "verbosity" through params will now more obviously be a way to manipulate global state. For example:

# before: WARN-level logs
# now:     WARN-level logs
lgb.train(dtrain, params={"verbosity": -1})

# before: INFO-level logs
# now:     WARN-level logs
lgb.train(dtrain)

This might confuse some people at first, but I think it's a better behavior .The most important thing to know: as of this PR, passing verbosity through params will always correctly set the log level.

@jameslamb jameslamb changed the title WIP: [python-package] respect 'verbose' setting when using custom objective function (fixes #6014) [python-package] respect 'verbose' setting when using custom objective function (fixes #6014) Jul 9, 2024
@jameslamb jameslamb marked this pull request as ready for review July 9, 2024 04:53
Copy link
Collaborator

@guolinke guolinke left a comment

Choose a reason for hiding this comment

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

Thank you

@jameslamb
Copy link
Collaborator Author

Thanks for the review @guolinke !

I'm excited about this one... I already feel that I've seen the volume of GitHub / Stack Overflow questions about managing LightGBM's verbosity reduced, hopefully this will reduce the volume of those reports even further.

@jameslamb jameslamb merged commit 2bc3ab8 into master Jul 12, 2024
41 checks passed
@jameslamb jameslamb deleted the fix/logging branch July 12, 2024 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[python-package] verbose does not supress warnings with custom objectives
2 participants