-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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] populate internal configs when creating Trainable through DistributedTrainableCreator #16128
Conversation
…ributedTrainableCreator
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.
Thanks a bunch for cleaning this up and adding tests! Looks good overall. Just left 1 comment.
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.
Nice work! Left 1 nit.
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.
Nice, LGTM!
Why are these changes needed?
DistributedTrainableCreator
creates a wrapperTrainable
which is used to create anImplicitFunc
. TheImplicitFunc
should be created with the same configs as theTrainable
, but is currently not becauseTrainable.__init__
clears out some of the internal configs (e.g. trial info).Changes
DistributedTrainable
class with abuild_config
method to populate internal configs.horovod
,tensorflow
,torch
} integrations to inherit fromDistributedTrainable
and callbuild_config
.Related issue number
Closes #15288
Checks
scripts/format.sh
to lint the changes in this PR.