Fix miles multi-node NCCL and verbs library resolution - #485
Open
zhouhelena1 wants to merge 8 commits into
Open
Conversation
|
Documentation preview: https://modal-labs-training-gym--training-gym-previews-preview-r-78da35.modal.run/485/docs |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Two root-caused library-resolution bugs in the Ray worker environment, affecting every multi-node miles run: - LD_LIBRARY_PATH put the system lib dir first, shadowing Modal's injected EFA stack (libfabric 1.30 + ofi NCCL plugin): fatal "Failed to initialize any NET plugin" on hosts pinning NCCL_NET_PLUGIN=ofi, and a silent slow-transport fallback elsewhere (~20x slower rollout generation measured). EFA dirs now precede the system dir. - On EFA hosts Modal bind-mounts the host's libibverbs over the system path, breaking the IBVERBS_PRIVATE_* coupling with the image's libmlx5, so mooncake's TransferEngine import fails. The image now snapshots its matched verbs set to /opt/gym-rdma/lib, and each node probes its system pair once: the prefix enters the worker search path only where the pair is broken. Unconditional inclusion dual-loads the verbs libs on healthy hosts and degrades NCCL IB 20-50x. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
zhouhelena1
force-pushed
the
helena/miles-infra-fixes
branch
from
September 1, 2026 19:23
b51440a to
74f3364
Compare
zhouhelena1
changed the title
Ray ships one env_vars mapping to every node, so the head's probe decided verbs resolution cluster-wide. The path now carries a fixed alias (/opt/gym-rdma/enabled) that each node symlinks to the copy only after its own probe fails; on healthy nodes the alias does not exist and the loader skips it. Also rebuilds the launcher change on top of current main, restoring apply_metric_image and cpu=miles.cpu, which the previous revision clobbered by starting from an older copy of the file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
zhouhelena1
added a commit
that referenced
this pull request
Sep 1, 2026
Ray ships one env_vars mapping to every node, so the head's probe decided verbs resolution cluster-wide; the path now carries a fixed alias that each node symlinks to the matched copy only after its own probe fails (matches PR #485). The resume guard now also validates .metadata-referenced shards when no shard files survived at all, instead of treating the directory as a non-torch_dist save. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment on lines
+32
to
+40
| NEW_USE = ( | ||
| " try:\n" | ||
| " from mooncake.engine import TransferEngine\n" | ||
| " except (ImportError, OSError) as exc:\n" | ||
| " raise RuntimeError(\n" | ||
| ' "p2p weight transfer requires mooncake\'s TransferEngine, "\n' | ||
| ' "which failed to import on this host (verbs stack mismatch)."\n' | ||
| " ) from exc\n" | ||
| " transfer_engine = TransferEngine()" |
Contributor
There was a problem hiding this comment.
🔍 Disaggregated P2P remains unproven
The final revision removed the matched-verbs snapshot and probe. NEW_USE still imports mooncake for P2P, so this topology needs EFA-host validation.
Was this helpful? React with 👍 or 👎 to provide feedback.
Contributor
Author
There was a problem hiding this comment.
Agreed on the validation boundary. The AWS validation run tall-degree-5d323e34b230 used Qwen3.5-4B on 2 x 8 H200s and exercised colocated Ray-IPC weight sync, not disaggregated Mooncake P2P. The root cause for that path was the unnecessary module-level Mooncake import: moving it to the P2P setup prevents colocated actors from loading the incompatible verbs dependency at all. Actual P2P still performs the real lazy TransferEngine import and raises a targeted error on incompatibility; it needs a separate EFA-host validation before we claim that topology works. I updated the PR description to state this explicitly.
7 tasks
zhouhelena1
added a commit
that referenced
this pull request
Sep 3, 2026
The recipe now pins cloud=oci (f01f1ae), so the two fixes carried in from helena/miles-infra-fixes (#485) no longer apply to any run it launches: - EFA-first LD_LIBRARY_PATH ordering — /opt/amazon/* does not exist on Mellanox hosts, so the entries were dead paths there. - The lazy mooncake TransferEngine import — the IBVERBS_PRIVATE break comes from Modal bind-mounting the host's libibverbs, which only happens on EFA hosts; mooncake imports cleanly on Mellanox. Both remain correct and belong in #485, which is the right vehicle for them once the EFA data-path hang is root-caused and these recipes can be unpinned. This PR should merge on its own without depending on that work. The _overlay_local_miles refactor from the same branch goes back to main's inline form; the re-apply list keeps only the patches this branch owns. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
tldr: previously, multinode miles on aws/efa with training gym launcher would fail because Ray workers loaded incompatible or incorrectly ordered EFA/libfabric/verbs libraries, which caused
plugin/net.cc:379 NCCL WARN Failed to initialize any NET pluginor unused Mooncake import failures.this PR fixes this by making the AWS EFA/OFI libraries take precedence and colocated workers avoid importing Mooncake.
validated this on a 2-node cluster pinned to aws. this issue was found during #460 , which now depends on this PR.
What changed
1. Keep Modal's AWS EFA/OFI stack authoritative in Ray workers
build_ray_runtime_envpreviously put/usr/lib/x86_64-linux-gnubefore Modal's injected EFA directories. NCCL's OFI plugin could therefore load the image's olderlibfabricinstead of/opt/amazon/efa/lib/libfabric.so.1, fail itsFABRIC_1.8requirement, and either abort withFailed to initialize any NET pluginor fall back to a much slower transport.The shared Ray
LD_LIBRARY_PATHnow always starts with:This is deliberately independent of a probe on the Ray head: on AWS every node resolves the complete host-injected EFA/verbs stack first, while on non-AWS hosts the absent directories are simply skipped by the loader. It also avoids mixing a copied image
libefa/libibverbsstack with host libraries, which producedEFA_*orIBVERBS_PRIVATE_*symbol-version failures.2. Remove Mooncake from the colocated actor import path
Miles imported
mooncake.engine.TransferEngineat module import time even though colocated weight sync uses Ray IPC, not Mooncake P2P. On affected EFA hosts that unused import could fail on the verbs ABI mismatch; catching the faileddlopenwas also unsafe because it could leave the process in a state that later aborted in glibc TLS initialization.The image patch now moves the Mooncake import into
setup_transfer_engine(). Colocated runs never load it; an actual disaggregated P2P run still imports it at point of use and raises a targeted error if the host verbs stack is incompatible.3. Make router bring-up tolerant of loaded clusters
Miles hardcoded a 30-second router bind deadline. The router child can exceed that while actors and imports contend during startup, so the image patch raises the deadline to 600 seconds while still returning as soon as the port is ready and still failing immediately if the child exits.
4. Preserve the fixes with
local_milesA local Miles checkout overlays
/root/milesafter the base-image patches run. The launcher now reapplies the Mooncake and router patches after that overlay, alongside the existing local-overlay patches, and emits targeted warnings when a custom checkout has intentionally diverged.Validation
tall-degree-5d323e34b230: Qwen3.5-4B Miles, pinned to AWS, 2 nodes × 8 H200s ineu-north-1, one rollout with 8 prompts × 4 samples and 128-token responses. It completed rollout generation, log-prob computation, optimizer step, checkpoint save, offload, and colocated weight sync with no retry, NCCL net-plugin failure, or glibc assertion.uv run pytest tests/test_miles_runtime_env.py tests/test_miles_patches.py -q— 14 passed, including EFA path precedence, lazy Mooncake import, and post-local_milespatch ordering.ruff checkandruff format --checkpassed for the changed launcher and tests.Scope
The AWS run validates the colocated Ray-IPC weight-sync path used by this configuration. It does not validate disaggregated Mooncake P2P on EFA; that path retains the real lazy
TransferEngineimport and needs a separate EFA-host run before it can be claimed as validated.