[feat] Extend aim.ext.tensorboard_tracker.run.Run to allow stdout logging and system stats and parameter logging
#2671
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ure terminal logs
…g occurs when enabled.
|
|
alansaul
commented
Apr 21, 2023
mihran113
changed the title
aim.ext.tensorboard_tracker.run.Run to allow stdout logging and system stats and parameter loggingaim.ext.tensorboard_tracker.run.Run to allow stdout logging and system stats and parameter logging
|
Everything looks good to me. Please just sign the CLA and add a CHANGELOG entry. |
mihran113
approved these changes
Apr 24, 2023
|
@alansaul could you please sign the CLA, so we can merge the enhancement? |
|
Apologies for the delay, I've merged upstream and signed the CLA, and fixed the changelog |
|
It looks like you might have some randomly failing tests as I don't believe I touched anything relating to the failing tests. |
mihran113
pushed a commit
that referenced
this pull request
May 30, 2023
…ogging and system stats and parameter logging (#2671) # Conflicts: # CHANGELOG.md
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implemented the simple approach to resolve #2670 by simply inheriting from
aim.sdk.run.Runrather thanaim.sdk.run.BasicRun, and we get the behaviour for free.Note: the default arguments for
system_tracking_interval,log_system_paramsandcapture_terminal_logsare different from the defaults provided byaim.sdk.run.Runto maintain backwards compatability, and by default not do any additional logging.I have also added a bunch of tests to check the extended behaviour works (default no capture, and that capturing each component works as expected). Perhaps there is an easier implementation for grabbing the metrics of interest, I might need some guidance as I am new to this project. I couldn't find anywhere where the logging behaviour of
aim.sdk.run.Runis captured to copy from. Let me know if you need any changes/think these tests should be removed.Resolves #2670