-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Fix how we set logging levels #3703
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
import logging # noqa I251 | ||
|
||
CRITICAL = logging.CRITICAL | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Imported here so that calling code doesn't need to |
||
FATAL = logging.FATAL | ||
ERROR = logging.ERROR | ||
WARNING = logging.WARNING | ||
INFO = logging.INFO | ||
DEBUG = logging.DEBUG | ||
NOTSET = logging.NOTSET | ||
|
||
_loggers = set() | ||
_log_level = NOTSET | ||
DATE_FORMAT = "%Y-%m-%d %H:%M:%S" | ||
LOG_FORMAT = "%(asctime)s %(levelname)s [%(filename)s:%(lineno)d] %(message)s" | ||
|
||
|
||
def get_logger(name: str) -> logging.Logger: | ||
""" | ||
Create a logger with the specified name. The logger will use the log level | ||
specified by set_log_level() | ||
""" | ||
logger = logging.getLogger(name=name) | ||
|
||
# If we've already set the log level, make sure new loggers use it | ||
if _log_level != NOTSET: | ||
logger.setLevel(_log_level) | ||
|
||
# Keep track of this logger so that we can change the log level later | ||
_loggers.add(logger) | ||
return logger | ||
|
||
|
||
def set_log_level(log_level: int) -> None: | ||
""" | ||
Set the ML-Agents logging level. This will also configure the logging format (if it hasn't already been set). | ||
""" | ||
global _log_level | ||
_log_level = log_level | ||
|
||
# Configure the log format. | ||
# In theory, this would be sufficient, but if another library calls logging.basicConfig | ||
# first, it doesn't have any effect. | ||
logging.basicConfig(level=_log_level, format=LOG_FORMAT, datefmt=DATE_FORMAT) | ||
|
||
for logger in _loggers: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In theory, if we use
here, and let the logger name hierarchy handle things, but I went for the explicit approach instead. |
||
logger.setLevel(log_level) |
This file was deleted.
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,5 +1,4 @@ | ||||||
# # Unity ML-Agents Toolkit | ||||||
import logging | ||||||
import argparse | ||||||
|
||||||
import os | ||||||
|
@@ -30,7 +29,9 @@ | |||||
from mlagents_envs.side_channel.engine_configuration_channel import EngineConfig | ||||||
from mlagents_envs.exception import UnityEnvironmentException | ||||||
from mlagents_envs.timers import hierarchical_timer, get_timer_tree | ||||||
from mlagents.logging_util import create_logger | ||||||
from mlagents_envs import logging_util | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to import everything ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I received the opposite feedback from @ervteng on Friday, see #3703 (comment) I think this is preferable when we want to use the logging levels; it's clearer in the code to see Another alternative is to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
logger = logging_util.get_logger(__name__) | ||||||
|
||||||
|
||||||
def _create_parser(): | ||||||
|
@@ -315,7 +316,7 @@ def write_timing_tree(summaries_dir: str, run_id: str) -> None: | |||||
with open(timing_path, "w") as f: | ||||||
json.dump(get_timer_tree(), f, indent=4) | ||||||
except FileNotFoundError: | ||||||
logging.warning( | ||||||
logger.warning( | ||||||
f"Unable to save to {timing_path}. Make sure the directory exists" | ||||||
) | ||||||
|
||||||
|
@@ -412,16 +413,16 @@ def run_cli(options: RunOptions) -> None: | |||||
print(get_version_string()) | ||||||
|
||||||
if options.debug: | ||||||
log_level = logging.DEBUG | ||||||
log_level = logging_util.DEBUG | ||||||
else: | ||||||
log_level = logging.INFO | ||||||
log_level = logging_util.INFO | ||||||
# disable noisy warnings from tensorflow | ||||||
tf_utils.set_warnings_enabled(False) | ||||||
|
||||||
trainer_logger = create_logger("mlagents.trainers", log_level) | ||||||
logging_util.set_log_level(log_level) | ||||||
|
||||||
trainer_logger.debug("Configuration for this run:") | ||||||
trainer_logger.debug(json.dumps(options._asdict(), indent=4)) | ||||||
logger.debug("Configuration for this run:") | ||||||
logger.debug(json.dumps(options._asdict(), indent=4)) | ||||||
|
||||||
run_seed = options.seed | ||||||
if options.cpu: | ||||||
|
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.
This is moved from
mlagents_envs/logging_util.py
and heavily modified.