-
Notifications
You must be signed in to change notification settings - Fork 3.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
[Model Parallel] Add configure sharded model hook #6679
Merged
Merged
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
9f8864f
Add base hook for model parallel
eac5344
fix callback signature
kaushikb11 32df0cb
Simplify hook
282a133
Add hook logic
7a94e72
add tests
kaushikb11 8091481
add property setter
kaushikb11 633fc77
add logic for being called once
kaushikb11 c99a36f
Update changelog
kaushikb11 a68c8d7
Merge branch 'master' into feat/model_parallel_hook
kaushikb11 9529a22
Fix
kaushikb11 3c1c782
fix return type
kaushikb11 a49ec3b
fix lambda callback test
kaushikb11 4dd55d7
Fix tests
kaushikb11 caad43c
Apply code suggestions
kaushikb11 a2574be
add logic for setup_optimizers_predispatch
kaushikb11 8c2bd6a
add common dummy model
kaushikb11 3240569
Swap call order
897bdbb
Remove test that isn't needed anymore
626fc7b
Update tests
kaushikb11 e94a7ae
Add a bit more doc
6a38417
Merge branch 'master' into feat/model_parallel_hook
202ef1a
Few code review fixes
0709baa
Update pytorch_lightning/accelerators/accelerator.py
SeanNaren 9152d08
Change hook name
fbfe65f
Fix test
bae858f
Test setup hook, refactor names
41e9c22
Swap call order of callbacks and model initialization
76c7376
Change name of context manager
2dcafd0
Merge branch 'master' into feat/model_parallel_hook
aa35583
add docstring
tchaton File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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.
@SeanNaren @kaushikb11 why's this needed in the trainer? could this be part of the accelerator setup? in L440 instead?
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.
which part exactly? the trainer has control over the hook, we do not want the hook to be called within the accelerator i think
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.
I see, on first read I thought that we could do this:
the upside is that more of the training type + accelerator logic lives inside of the accelerator internals
but the downside is that this splits the callback hook from the sharded model setup which can be less convenient
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.
Agree as long as we're ok with calling the model hook within the accelerator (cc @justusschock @awaelchli)