-
Notifications
You must be signed in to change notification settings - Fork 62
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
Multi-node training support #440
Conversation
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
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.
Multi node is huge, thanks for the work 🎊 . The PR looks good to me, just left some minor nits. Wonder if @michaelbenayoun if it's possible to test the least functionality with CIs or other paths(I know setting up for multiple nodes is tricky...
@@ -615,6 +670,34 @@ def parameter_can_be_initialized(model: torch.nn.Module, parent_module: torch.nn | |||
) | |||
|
|||
|
|||
@requires_neuronx_distributed | |||
def apply_activation_checkpointing( | |||
model: Union[torch.nn.Module, "NxDPPModel"], |
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.
No need for quote neither
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.
Here it is needed since if neuronx_distributed
is not available it will not be imported.
We test settings with TP + PP, but testing the multi-node setting is more tricky, it requires to create a node cluster, etc. For now let's keep it like that. |
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 only looked at the part that is related to the cache: I would rather move the import inside the utils to avoid multiple imports.
Merging, tested on custom instances. |
@michaelbenayoun can we conduct multi-node training with Mistral? |
Yes it should be! |
This PR adds support for multi-node training.
llama-7b
llama-70b
In coming PRs:
neuronx_distributed