-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[MetaSchedule] Logging Interface Unification #11157
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
[MetaSchedule] Logging Interface Unification #11157
Conversation
|
Thanks for massively enhancing the current logging system! However, on the implementation, we do need to follow the standard best practice in python, namely, using: logger = logging.getLogger(__name__)to get a logger for this file. |
|
Thanks for the careful review, I've fixed all the logger usage and made sure it follows the current recommended practice. Ready for review. |
python/tvm/meta_schedule/tune.py
Outdated
| # pylint: disable=import-outside-toplevel | ||
| import logging | ||
| import os.path | ||
| from pathlib import Path |
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.
Pathlib is definitely a good library, but perhaps it's less plausible if we mix os.path and Pathlib. In our particular case, shall we just do:
os.makedirs(path, exist_ok=False)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.
I think it might be acceptable to use exist_ok = True in this case cause the logs folder could be reused.
d49fa1f to
feb2bfe
Compare
| if (!opt_max_threads_per_block.defined()) { | ||
| LOG(WARNING) << "Target does not have attribute \"max_threads_per_block\", therefore the " | ||
| "rule CrossThreadReduction will not be applied"; | ||
| TVM_PY_LOG(WARNING, context->logging_func) |
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.
I was thinking if it's better to make it a LOG(FATAL) instead, but it's do that in a separate PR
e9a1d8c to
2863996
Compare
|
Any updates? |
junrushao
left a comment
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! Please try it out for the last time and confirm it prints the dates properly, and then feel free to merge it yourself
|
Thanks @junrushao1994 @Hzfengsy for reviewing! |
* Implement new logging interface. * Major interface usage update. * Functionality fix. * Switch logging conditions. * Tweak logging interface. * Minor fix. * Feature updates. * Logging usage. * Linting. * Fix linting. * Fix handler type. * Fix issues. * Nits. * Address issues. * Add DEBUG level fall back. * Minor fixes. * Allow parameterized configuration. * Linting. * Polish interface.
* Implement new logging interface. * Major interface usage update. * Functionality fix. * Switch logging conditions. * Tweak logging interface. * Minor fix. * Feature updates. * Logging usage. * Linting. * Fix linting. * Fix handler type. * Fix issues. * Nits. * Address issues. * Add DEBUG level fall back. * Minor fixes. * Allow parameterized configuration. * Linting. * Polish interface.
This PR introduces a new interface to unify meta schedule logging from both c++ and python side. It will be easier to control logging level and format content. From now on, only task scheduler level will be printed to screen by default, and task level information will be logged to a file in given folder. We still support any logging configuration from outside, and use
tvm.meta_scheduleas the main logger.The interface uses logger on the python side while on the c++ side we created a new macro called
TVM_PY_LOGto work int the following style wherelogging_funcis the optional packed function for logging (thanks to #10032 ):The logger would fall back to original c++ logger function
LOG(Level)if given logging function is not defined.Recomended setup of logger is as follows:
And further configuration of task level loggers can be set in
TuneConfigfor features like whether to stream to screen, whether to propagate to main logger, patterns, etc, by passing a Dict toTuneConfigas follows:Default setting is to output to files in
log_dirfolder and the logging structure looks like the following directory:Suggestions are welcome!
CC: @junrushao1994 @shingjan @comaniac