Repository navigation
Add sync_dist_op to LightningModule logging - #21892
zupengwang wants to merge 2 commits into
Conversation
Signed-off-by: Zupeng Wang <71580390+zupengwang@users.noreply.github.com>
|
Hi @tchaton, I pushed the reviewed follow-up The epoch-mean path now applies an explicitly requested rank reduction to the local batch-weighted means. Previously the operation was applied separately to the weighted sums and sample counts: for local means of 12 and 3.5, explicit On a remote Linux host with Python 3.12.13 and torch 2.12.1+cpu using two-rank Gloo, the same added regressions give 9 failures / 2 passes / 2 skips with the previous head's unchanged production code and 11 passes / 2 skips with this commit. The related five-file logging/metric suite gives 74 passes / 8 GPU-dependent skips, and all 11 applicable pre-commit hooks pass. A separate conflict-free integration with master Could you review the current commit and this explicit-operation/default-operation distinction? Read the Docs has passed on this head. The new Package, Probot, Code check, Docs builds, Check Schema, and Test PyTorch workflows all show |
What does this PR do?
Adds a
sync_dist_opargument toLightningModule.logandLightningModule.log_dictso the reduction across distributed ranks can be configured independently from the reduction across epoch steps.When
sync_dist_opis omitted, the distributed operation still falls back toreduce_fx, preserving existing behavior. The argument is appended to the public signatures to avoid shifting existing positional arguments. The API docstrings and PyTorch changelog are updated.The regression test covers CPU, single-GPU, two-process CPU DDP, and two-GPU DDP. It logs rank-dependent values with
sync_dist_op="sum"andreduce_fx="max", verifying the two reduction stages produce the expected metric.Validation:
python -m pytest -q tests/tests_pytorch/trainer/logging_/test_train_loop_logging.py::test_logging_sync_dist_true(4 passed)CUDA_VISIBLE_DEVICES= python -m pytest -q tests/tests_pytorch/core/test_results.py tests/tests_pytorch/trainer/logging_/test_train_loop_logging.py(31 passed, 4 skipped)PL_RUN_STANDALONE_TESTS=1 python -m pytest -q tests/tests_pytorch/strategies/test_ddp_integration.py::test_ddp_gradients_synced[...](all four automatic/manual and static/non-static variants passed when run individually)ruff checkandruff format --checkon the changed Python filesmypyon the changed source filesuv build --offline --wheelfor thepytorchpackageAI assistance disclosure: AI tools assisted with implementation and test preparation. The contributor reviewed the complete diff and validated the change on CPU and GPU.
Fixes #17831
No breaking changes are intended. The default distributed reduction remains unchanged.
Before submitting
strategy.reducevsreduce_fx#17831.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