Refactor launch for greater extensibility
#1123
Merged
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
Signed-off-by: Antoni Baum <antoni.baum@protonmail.com>
|
The documentation is not available anymore as the PR was closed or merged. |
sgugger
approved these changes
Feb 28, 2023
There was a problem hiding this comment.
Thanks a lot for your PR! This looks great to me. Mind having a second look @muellerzr ?
Can you just solve the merge conflict @Yard1 ?
muellerzr
approved these changes
Feb 28, 2023
Signed-off-by: Antoni Baum <antoni.baum@protonmail.com>
Signed-off-by: Antoni Baum <antoni.baum@protonmail.com>
|
@muellerzr Thanks, should be good now! |
dbpprt
reviewed
Mar 1, 2023
| @@ -23,8 +23,9 @@ | |||
|
|
|||
| from ..commands.config.config_args import SageMakerConfig | |||
| from ..commands.config.config_utils import DYNAMO_BACKENDS | |||
| from ..utils import DynamoBackend, PrecisionType, is_torch_version, merge_dicts | |||
There was a problem hiding this comment.
Ideally it should be imported from ..utils and not using the direct path to .other, following the same mechanism used for all other utils, isn't it?
There was a problem hiding this comment.
That results in a circular import, unfortunately
|
FYI @natolambert this was the PR that i had mentioned to you ages ago. It'll be useful if you guys plan to do sweeps in TRL. |
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.
This PR refactors
commands.launchto break up the logic into smaller functions, allowing for the environment preparation logic and actual launch logic to be decoupled. Therefore, a developer can now prepare an environment dict (and command list, where applicable) without having to launch any processes.There have been no changes to the logic - this is purely a refactor of an existing structure.
As discussed offline, this will unblock Ray Train and other distributed frameworks which launch their own jobs.