feat(rollout): integrate MInf ledger capture with checkpointing - #3869
feat(rollout): integrate MInf ledger capture with checkpointing#3869lauradang wants to merge 11 commits into
Conversation
Signed-off-by: Teodor-Dumitru Ene <teodord.ene@gmail.com>
Signed-off-by: Teodor-Dumitru Ene <teodord.ene@gmail.com>
Signed-off-by: Teodor-Dumitru Ene <teodord.ene@gmail.com>
Signed-off-by: Teodor-Dumitru Ene <teodord.ene@gmail.com>
Signed-off-by: Teodor-Dumitru Ene <teodord.ene@gmail.com>
Signed-off-by: Laura Dang <laurad@nvidia.com>
Signed-off-by: Laura Dang <laurad@nvidia.com>
Signed-off-by: Laura Dang <laurad@nvidia.com>
|
/ok to test 0ac1707 |
There was a problem hiding this comment.
Review of the lauradang commits only (08faaee37, 0c5e033ec, 0ac170793). Verified against the pinned Gym submodule (5cd5c0d), the pinned Megatron-Bridge → Megatron-LM (0c565c9a0 → d12f6c8c9), and the tdene:Megatron-LM:tde/ledger_capture branch. 4 inline comments below, plus one Gym-side issue on the submodule bump that I can't anchor inline:
Also verified OK, for the record: the exactly-one-rank expectation in flush_token_capture matches the MInf branch (ledger is populated only on the MP-coordinator rank per DP engine; fan-out-and-union is the documented pattern), and set_generation_epoch from global rank 0 propagates to all engines via the DP coordinator.
| ] | ||
| if missing: | ||
| raise RuntimeError( | ||
| "Megatron token capture requires the MInf ledger-capture API; " |
There was a problem hiding this comment.
[DEP/blocker] The pinned Megatron-Bridge (0c565c9a0 → Megatron-LM d12f6c8c9) has no local_metadata_ledger_* / fetch_from_metadata_ledger API (verified by git-grep at the pinned commits; only set_generation_epoch exists there). So with this PR as-is, enabling megatron token capture always hits this raise at setup. Failing loudly here is right, and the PR body declares the tdene:Megatron-LM:tde/ledger_capture dependency — but the PR doesn't bump the Megatron-Bridge pin, so this needs the MInf ledger to land and the submodule bump to be part of (or precede) the merge. Flagging so the merge order is explicit.
There was a problem hiding this comment.
Pending merge: https://github.com/tdene/Megatron-LM/tree/tde/ledger_capture
Signed-off-by: Laura Dang <laurad@nvidia.com>
Summary
Dependencies
amahishi/partial-rollout-telemetry)tdene:Megatron-LM:tde/ledger_captureTesting
Local test limitation
The RL lockfile supports Linux x86_64/aarch64 only, so the focused RL pytest files cannot run natively on this macOS host. An ephemeral dependency run progressed through PyTorch, Transformers, and pyzmq before reaching the NVIDIA-only
pynvmlruntime dependency. The PR remains draft pending Linux CI.