-
Notifications
You must be signed in to change notification settings - Fork 223
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Enhanced monitor with system metrics logging and wandb support #90
Conversation
…upload/download bytes, convergence rounds and time
…ed or normally terminated)
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. And please see the inline comments.
): | ||
log_res_best[ | ||
f"{final_type}/{inner_key}"] = inner_val | ||
# log_res_best = {} |
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.
What are the commented codes for?
except ImportError: | ||
logger.error( | ||
"cfg.wandb.use=True but not install the wandb package") | ||
exit() |
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.
How about raising ValueError?
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.
If import wandb
failed, the system will trigger ImportError
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.
Approved! BTW, I noticed that several customized trainers have to explicitly register the newly added hook (seems to estimate flops). I am curious about when we should do that for a new trainer (e.g., the newly added FLIT by Haohui).
Enhanced monitor with system metrics logging and wandb support: