Fix progress bar deadlock on DDP metrics computation. #21322

tchittesh · 2025-10-29T05:49:50Z

What does this PR do?

Before submitting

Was this discussed/agreed via a GitHub issue? (not for typos and docs)
Did you read the contributor guideline, Pull Request section?
Did you make sure your PR does only one thing, instead of bundling different changes together?
Did you make sure to update the documentation with your changes? (if necessary)
Did you write any new necessary tests? (not for typos and docs)
Did you verify new and existing tests pass locally with your changes?
Did you list all the breaking changes introduced by this pull request?
Did you update the CHANGELOG? (not for typos, docs, test updates, or minor internal changes/refactors)

PR review

Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the review guidelines. In short, see the following bullet-list:

Reviewer checklist

Is this pull request ready for review? (if not, please submit in draft mode)
Check that all items from Before submitting are resolved
Make sure the title is self-explanatory and the description concisely explains the PR
Add labels and milestones (and optionally projects) to the PR so it can be classified

📚 Documentation preview 📚: https://pytorch-lightning--21322.org.readthedocs.build/en/21322/

SkafteNicki · 2025-12-01T05:53:14Z

tests/tests_pytorch/callbacks/progress/test_rich_progress_bar.py

+
+    We used to have a bug where metrics were synced only on the rank 0 process. See
+    https://github.com/Lightning-AI/pytorch-lightning/issues/21264
+    for more details.
+


Suggested change

We used to have a bug where metrics were synced only on the rank 0 process. See

https://github.com/Lightning-AI/pytorch-lightning/issues/21264

for more details.

SkafteNicki · 2025-12-01T05:54:13Z

tests/tests_pytorch/callbacks/progress/test_rich_progress_bar.py

+    for more details.
+
+    """
+    RichProgressBar()


Suggested change

RichProgressBar()

seem to not really be used?

SkafteNicki · 2025-12-01T05:54:27Z

tests/tests_pytorch/callbacks/progress/test_tqdm_progress_bar.py

+
+    We used to have a bug where metrics were synced only on the rank 0 process. See
+    https://github.com/Lightning-AI/pytorch-lightning/issues/21264
+    for more details.
+


Suggested change

We used to have a bug where metrics were synced only on the rank 0 process. See

https://github.com/Lightning-AI/pytorch-lightning/issues/21264

for more details.

Copilot

Pull request overview

This PR fixes a deadlock issue in DDP (Distributed Data Parallel) training when progress bars compute metrics at the end of a training epoch. The bug occurred because metrics synchronization only happened on rank 0 processes, causing other ranks to wait indefinitely.

Key Changes:

Modified TQDMProgressBar and RichProgressBar to call get_metrics() unconditionally on all ranks before checking if the progress bar is enabled/disabled
Added comprehensive tests for both progress bar types to verify DDP deadlock scenarios are resolved

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
`src/lightning/pytorch/callbacks/progress/tqdm_progress.py`	Moved `get_metrics()` call outside the conditional check to ensure all DDP ranks participate in metric synchronization
`src/lightning/pytorch/callbacks/progress/rich_progress.py`	Moved `get_metrics()` call before the early return check to ensure synchronization happens on all ranks
`tests/tests_pytorch/callbacks/progress/test_tqdm_progress_bar.py`	Added DDP deadlock test with proper mocking to isolate the progress bar fix
`tests/tests_pytorch/callbacks/progress/test_rich_progress_bar.py`	Added DDP deadlock test for RichProgressBar (contains issues that need fixing)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-01T05:56:08Z

src/lightning/pytorch/callbacks/progress/rich_progress.py

        if self._metric_component:
            self._metric_component.update(metrics)


The condition if self._metric_component: is redundant. The previous check on line 655 already ensures that self._metric_component is not None when this line is reached. This check can be simplified or removed.

Suggested change

if self._metric_component:

self._metric_component.update(metrics)

self._metric_component.update(metrics)

Copilot · 2025-12-01T05:56:09Z

tests/tests_pytorch/callbacks/progress/test_rich_progress_bar.py

+    model = MyModel()
+
+    # We need to mock these logger connector hooks, since these also attempt to sync metrics
+    # and can "save" otherwise incorrect implementations of TQDMProgressBar.on_train_epoch_end.


The comment mentions 'TQDMProgressBar' but this test is for RichProgressBar. The comment should reference RichProgressBar instead.

Suggested change

# and can "save" otherwise incorrect implementations of TQDMProgressBar.on_train_epoch_end.

# and can "save" otherwise incorrect implementations of RichProgressBar.on_train_epoch_end.

Copilot · 2025-12-01T05:56:09Z

tests/tests_pytorch/callbacks/progress/test_rich_progress_bar.py

+                process_group_backend="gloo",  # run on CPU
+                timeout=datetime.timedelta(seconds=5),  # timeout quickly for the test to fail
+            ),
+            enable_progress_bar=True,


The RichProgressBar instance created on line 620 is not passed to the trainer's callbacks. This test should include callbacks=[pbar] in the Trainer initialization to actually test the RichProgressBar implementation. Without this, the test only uses the default progress bar behavior and doesn't exercise the fix in RichProgressBar._update_metrics().

Copilot · 2025-12-01T05:56:09Z

tests/tests_pytorch/callbacks/progress/test_rich_progress_bar.py

    # This should not raise an AssertionError
    trainer.fit(model)
+
+


This test should have the @RunIf(rich=True) decorator to ensure it only runs when the rich library is available, since it instantiates RichProgressBar which raises ModuleNotFoundError when rich is not installed.

Suggested change

@RunIf(rich=True)

github-actions bot added the pl Generic label for PyTorch Lightning package label Oct 29, 2025

tchittesh marked this pull request as ready for review October 31, 2025 03:27

tchittesh requested review from Borda, ethanwharris, justusschock, lantiga and tchaton as code owners October 31, 2025 03:27

bhimrazy added the progress bar: rich label Nov 4, 2025

tchittesh force-pushed the bugfix/21264_pbardeadlock branch from c1c55c9 to 77bfef4 Compare November 9, 2025 03:11

Fix progress bar deadlock on DDP metrics computation.

a1a3700

tchittesh force-pushed the bugfix/21264_pbardeadlock branch from 77bfef4 to a1a3700 Compare November 9, 2025 03:12

tchittesh mentioned this pull request Nov 10, 2025

TQDMProgressBar bug (?) causes DDP to hang #21264

Open

SkafteNicki requested a review from Copilot December 1, 2025 05:52

Merge branch 'master' into bugfix/21264_pbardeadlock

5ea69dd

Copilot started reviewing on behalf of SkafteNicki December 1, 2025 05:52 View session

SkafteNicki reviewed Dec 1, 2025

View reviewed changes

Copilot finished reviewing on behalf of SkafteNicki December 1, 2025 05:55

Copilot AI reviewed Dec 1, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix progress bar deadlock on DDP metrics computation. #21322

Fix progress bar deadlock on DDP metrics computation. #21322

tchittesh commented Oct 29, 2025 •

edited by github-actions bot

Loading

Uh oh!

SkafteNicki Dec 1, 2025

Uh oh!

SkafteNicki Dec 1, 2025

Uh oh!

SkafteNicki Dec 1, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		if self._metric_component:
		self._metric_component.update(metrics)

	# and can "save" otherwise incorrect implementations of TQDMProgressBar.on_train_epoch_end.
	# and can "save" otherwise incorrect implementations of RichProgressBar.on_train_epoch_end.

Fix progress bar deadlock on DDP metrics computation. #21322

Are you sure you want to change the base?

Fix progress bar deadlock on DDP metrics computation. #21322

Conversation

tchittesh commented Oct 29, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

PR review

Uh oh!

SkafteNicki Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

SkafteNicki Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

SkafteNicki Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tchittesh commented Oct 29, 2025 •

edited by github-actions bot

Loading