Skip to content

Commit c267634

Browse files
tushar00jainmeta-codesync[bot]
authored andcommitted
Fix cpu_offloaded_metric_module type-check for new_group return (#4457)
Summary: Pull Request resolved: #4457 Removing the TorchComms `new_group` -> `split_group` delegation in `distributed_c10d.py` (D113127841 / pytorch#189071) leaves `_new_group_with_tag` as `new_group`'s only return path. `new_group` has no explicit return annotation, so Pyrefly now infers `ProcessGroup | int | None` (the `int` is the `GroupMember.NON_GROUP_MEMBER == -100` sentinel) instead of the narrower type it used to see. `cpu_offloaded_metric_module.py` assigns `dist.new_group(backend="gloo")` straight into an attribute typed `Optional[ProcessGroup]`, which no longer type-checks: cpu_offloaded_metric_module.py:303 `ProcessGroup | int | None` is not assignable to attribute `cpu_process_group` with type `ProcessGroup | None` Both call sites pass `ranks=None`, so every rank is a member and the result is always a real `ProcessGroup` (never `NON_GROUP_MEMBER`); narrow the value with `cast`. This is an internal-only, monorepo-coexistence break: OSS pytorch CI does not type-check torchrec against the new signature, which is why pytorch#189071 was green on GitHub. Kept as a separate diff so the DiffTrain import stays byte-identical to GitHub. Reviewed By: dboyda Differential Revision: D113289735 fbshipit-source-id: 0f3dba9c0fd80fced2902dd0bc507f4704af4291
1 parent c05a4d7 commit c267634

1 file changed

Lines changed: 9 additions & 4 deletions

File tree

torchrec/metrics/cpu_offloaded_metric_module.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
import threading
1616
import time
1717
import traceback
18-
from typing import Any, Dict, Mapping, Optional, Union
18+
from typing import Any, cast, Dict, Mapping, Optional, Union
1919

2020
import torch
2121
from torch import distributed as dist
@@ -298,9 +298,12 @@ def __init__(
298298

299299
# Created lazily on first compute so the module can be constructed without
300300
# an initialized process group (e.g. single-process model validation).
301-
# pyrefly: ignore
301+
# new_group returns a real ProcessGroup here (ranks=None => every rank is a
302+
# member, never NON_GROUP_MEMBER); narrow away the int/None sentinels.
302303
self.cpu_process_group: Optional[dist.ProcessGroup] = (
303-
dist.new_group(backend="gloo") if dist.is_initialized() else None
304+
cast(Optional[dist.ProcessGroup], dist.new_group(backend="gloo"))
305+
if dist.is_initialized()
306+
else None
304307
)
305308
self.comms_module: CPUCommsRecMetricModule = CPUCommsRecMetricModule(
306309
*args,
@@ -755,7 +758,9 @@ def _process_metric_compute_job(
755758
# Manual distributed sync (replaces TorchMetrics.metric.Metric.sync())
756759
all_gather_start_ms = time.time()
757760
if self.cpu_process_group is None:
758-
self.cpu_process_group = dist.new_group(backend="gloo")
761+
self.cpu_process_group = cast(
762+
dist.ProcessGroup, dist.new_group(backend="gloo")
763+
)
759764
aggregated_states = self.comms_module.get_pre_compute_states(
760765
self.cpu_process_group
761766
)

0 commit comments

Comments
 (0)