fix(launcher): honor --nproc-per-node over the local device count - #3775
Open
kabirvashisht4-glitch wants to merge 2 commits into
Open
kabirvashisht4-glitch wants to merge 2 commits into
kabirvashisht4-glitch wants to merge 2 commits into
Conversation
InteractiveLauncher.launch probed the local GPU count before acting on an explicit --nproc-per-node, so the flag lost to the probe twice over. On a one-GPU host `num_devices == 1` short-circuited to the in-process path, so `--nproc-per-node 4` silently started one worker instead of four. And `determine_local_world_size(nproc_per_node="gpu")` raises when no CUDA device is visible, so `--nproc-per-node 1` failed inside torch before the in-process branch that needs no device count could run. Probe only when no worker count was requested, and pick the single-process path from the resolved count. Behaviour without the flag is unchanged. Signed-off-by: kabirvashisht4-glitch <kabirvashisht4@gmail.com>
Contributor
|
/ok to test a0989c3 |
Contributor
|
/ok to test e445bb0 |
Contributor
|
/ok to test e445bb0 |
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do ?
Makes
InteractiveLauncheract on an explicit--nproc-per-nodeinstead ofletting the local GPU probe override it. The probe ran first, so the flag lost
to it in two ways:
1.
--nproc-per-node Nwas silently downgraded on a one-GPU host.num_devices == 1short-circuits to the in-process path whatever the userasked for, so
automodel config.yaml --nproc-per-node 4started one workerand logged "Launching job locally on a single device" without saying the
requested value was dropped. Running several ranks on one device is a normal way
to exercise FSDP/DDP or PP code paths on a dev box, and
torchrunsupports it.2.
--nproc-per-node 1failed with no visible CUDA device.determine_local_world_size(nproc_per_node="gpu")raises whentorch.cuda.is_available()is false, and it ran before thenproc_per_node == 1branch — the one path that needs no device count, since it runs the recipe
in-process:
Also fires under
CUDA_VISIBLE_DEVICES="". The error comes from insidetorch.distributed.run, so it names neither the flag nor an AutoModelrequirement.
Scope, to be explicit: this does not make CPU training work — the recipe
still requires CUDA further down. It makes the documented flag behave as
documented and lets the failure come from the framework rather than from a
device probe the requested single-process path never needed.
Changelog
InteractiveLauncher.launchprobesdetermine_local_world_sizeonly whennproc_per_node is None; an explicit value is used as-is.nproc_per_node == 1 or num_devices == 1, so an explicitN > 1reachestorchrun even when one device is visible.
tests/unit_tests/launcher/test_interactive_launcher.py:explicit
4on a one-device host reaches torchrun with 4 workers; an explicitcount never calls the probe; and
--nproc-per-node 1still runs in-processwhen the probe would raise
ValueError("Cuda is not available.").Before your PR is "Ready for review"
Pre checks:
--helpalready documents the flag as "Number of workers per node forlocal/interactive jobs" with no mention of clamping to the device count, so the
code now matches it and no doc change was needed.
Compatibility. Without the flag the behaviour is byte-for-byte the same: the
probe still runs, still asserts
> 0, and still picks in-process for one deviceor torchrun for more. All 21 pre-existing tests in the file pass unmodified,
including
test_interactive_launcher_single_device,test_interactive_launcher_multi_device, andtest_interactive_launcher_multi_device_explicit_nproc.Verification (CPU, no GPU needed):
pytest tests/unit_tests/launcher/→ 103 passed.interactive.pymakes all three new tests fail, so they arenot vacuous.
pytest tests/unit_tests/launcher/ tests/unit_tests/_cli/ tests/unit_tests/recipes/→ 1291 passed, 12 skipped.(
tests/unit_tests/recipes/dllmexcluded locally: it fails to importtransformers.models.diffusion_gemmaonmaintoo, unrelated to this change.)ruff format/ruff checkclean.Additional Information