-
Notifications
You must be signed in to change notification settings - Fork 176
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
Do not log caller for INFO messages #418
Conversation
Codecov Report
@@ Coverage Diff @@
## master #418 +/- ##
==========================================
+ Coverage 83.42% 83.68% +0.25%
==========================================
Files 77 77
Lines 7790 7795 +5
==========================================
+ Hits 6499 6523 +24
+ Misses 1291 1272 -19
|
db0ae23
to
ea40b98
Compare
@@ -461,7 +461,7 @@ def _update_global_epoch(self, grad_scaler: Optional[GradScaler]) -> None: | |||
if not self.client_mode: | |||
self.grad_averager.state_sharing_priority = self.local_epoch | |||
|
|||
logger.log(self.status_loglevel, f"Transitioning to epoch {self.local_epoch}.") | |||
logger.debug(f"Transitioning to epoch {self.local_epoch}") |
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 duplicates the Step N
message (see the "After" screenshot).
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 message is useful because
- it signifies when a (potentially long) optimizer step is finished
- it is the only message that will be printed when use_local_updates=True (i.e. there is no global step)
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.
Good job!
Please revert the epoch message though (or continue the discussion)
e1c3cd4
to
ea40b98
Compare
The caller info for INFO log messages is often long but rarely useful. Such long lines confuse new users.
This PR hides the caller info for INFO messages only, unless a user specifies env variable
HIVEMIND_ALWAYS_LOG_CALLER=True
(True
is case-insensitive).I think it is important that this behavior does not depend on
HIVEMIND_LOGLEVEL
. Example: a user could create a Colab notebook parsing the logs and change theHIVEMIND_LOGLEVEL
in the last minute, thus breaking the parser (since the INFO messages start to have the other format).Before:
After: