Use device names, not rank ids, for device placement in test helpers - #8397
Conversation
get_accelerator().current_device() returns a CUDA device index on GPU
backends, but on CPU it is the LOCAL_RANK environment value, a plain
string like '1' that torch rejects as a device ("Invalid device
string"). reduce_boolean_flags and the autotp tests fed that value
straight into tensor/device placement, so multi-rank CPU runs failed
before reaching their first collective.
Use current_device_name() instead: it yields a full device string on
every backend ('cpu', 'cuda:N', ...) and is equivalent to the index on
GPU backends. Also carry the boolean flag in a 1-dim tensor because
gloo rejects 0-dim inputs to all_gather_into_tensor.
Validated as part of the multi-rank CPU experiment in deepspeedai#8381: this
failure class disappeared with zero regressions vs the same-commit
baseline.
Signed-off-by: Guokai Ma <guokai.ma@intel.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ceca37333
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| device = get_accelerator().current_device() | ||
| tensor_flag = torch.tensor(1 if flag else 0, dtype=torch.int, device=device) | ||
| # current_device() is a rank id on CPU, not a valid torch device; use the device name. | ||
| device = get_accelerator().current_device_name() |
There was a problem hiding this comment.
This is a non-merge commit, but its message has no Signed-off-by trailer, so it violates the repository's mandatory commit requirement and may be rejected by DCO/CI checks. Recreate the commit with --signoff using the configured Git identity.
AGENTS.md reference: AGENTS.md:L8-L8
Useful? React with 👍 / 👎.
The reference-input randn() call exceeds the 119-column limit, failing the formatting CI. Split one argument per line as yapf requires. Signed-off-by: Ma, Guokai <guokai.ma@intel.com>
2 participants
Problem
get_accelerator().current_device()returns a device index on GPU backends (torch.cuda.current_device()→ int), but on CPU it returns theLOCAL_RANKenvironment value — a plain string like'1'. Two test-side consumers fed that value straight into tensor/device placement:reduce_boolean_flagsintests/unit/common.py(backbone ofallclose_on_all_ranks, the "all ranks succeed or fail together" check)tests/unit/v1/autotp/test_autotp_training.pyOn CPU this fails immediately with
RuntimeError: Invalid device string: '1'— before the first collective even runs.Change
current_device_name(), which returns a full device string on every backend ('cpu','cuda:N','mps:0', …) and is equivalent to the index on GPU backends.reduce_boolean_flags, carry the flag in a 1-dim tensor: gloo rejects 0-dim inputs toall_gather_into_tensor(NCCL tolerates them), so the previous form would have failed on the very next line.Validation
Validated as part of the multi-rank CPU CI experiment in #8381 (same-commit baseline comparison): this failure class disappeared, zero regressions on previously-passing tests.