-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
[tune] Fix analysis without registered trainable #21475
[tune] Fix analysis without registered trainable #21475
Conversation
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.
Looks great, just have the same question as @matthewdeng
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.
Tests fail atm, this should fix it
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.
Looks good, thanks!
Why are these changes needed?
This PR fixes issues with loading
ExperimentAnalysis
from path or pickle if the trainable used in the trials is not registered. Chiefly, it ensures that thestub
attribute set inload_trials_from_experiment_checkpoint
doesn't get overridden by the state of the loaded trial, and that when pickling, all trials inExperimentAnalysis
are turned into stubs if they aren't already. A test has also been added.Related issue number
Closes #21212
Checks
scripts/format.sh
to lint the changes in this PR.