-
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
Fixes around Strategy.set_world_ranks
#16966
Conversation
@@ -211,13 +211,10 @@ def reduce( | |||
|
|||
def setup_distributed(self) -> None: | |||
self._launched = True | |||
self.set_world_ranks() | |||
rank_zero_only.rank = self.global_rank | |||
|
|||
def set_world_ranks(self) -> None: |
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.
The accelerator connector will call this method. But we don't want the XLA strategy setting the rank (yet). Since XLAStrategy inherits from DDPStrategy, we need to leave it empty here.
@@ -211,13 +211,10 @@ def reduce( | |||
|
|||
def setup_distributed(self) -> None: | |||
self._launched = True | |||
self.set_world_ranks() |
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.
the line below already covers what was done in this method.
set_world_ranks
in main process for XLA strategyset_world_ranks
in main process for XLA strategy
set_world_ranks
in main process for XLA strategyStrategy.set_world_ranks
⛈️ Required checks status: Has failure 🔴
Groups summary🟢 pytorch_lightning: Tests workflow
These checks are required after the changes to 🟢 pytorch_lightning: Azure GPU
These checks are required after the changes to 🟢 fabric: Docs
These checks are required after the changes to 🟢 pytorch_lightning: Docs
These checks are required after the changes to 🟢 lightning_fabric: CPU workflowThese checks are required after the changes to 🟢 lightning_fabric: Azure GPU
These checks are required after the changes to 🟢 mypy
These checks are required after the changes to 🔴 installThese checks are required after the changes to 🟢 link-check
These checks are required after the changes to Thank you for your contribution! 💜
|
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.
The added changes LGTM @carmocca
* don't call set_world_ranks in xla strategy * update * fabric and other strategies * CHANGELOG * Typos * Reuse test --------- Co-authored-by: Carlos Mocholí <[email protected]> (cherry picked from commit 50662eb)
* don't call set_world_ranks in xla strategy * update * fabric and other strategies * CHANGELOG * Typos * Reuse test --------- Co-authored-by: Carlos Mocholí <[email protected]> (cherry picked from commit 50662eb)
What does this PR do?
Avoid calling set_world_ranks for XLAStrategy.
cc @Borda @carmocca @justusschock @awaelchli @JackCaoG @steventk-g @Liyang90