Extend dynamic inference asynchronous scheduling support - #5939
Conversation
Signed-off-by: Lawrence McAfee <lmcafee@nvidia.com>
Signed-off-by: Lawrence McAfee <lmcafee@nvidia.com>
Signed-off-by: Lawrence McAfee <lmcafee@nvidia.com>
|
The name
The real common thread is that all three flush the in-flight forward and issue the next one after resolve/admit. Recommendation.
Engine flow becomes
I have left a separate comment about the "5 counter restore hack", which we can avoid with a simple fix. |
|
/claude strict-review |
There was a problem hiding this comment.
Strict review passed — no significant issues found. LGTM
Reviewed for implementation correctness, distributed correctness (TP/PP/SP/CP/EP), performance, and backward compatibility. Key paths verified:
- MTP sequence-length termination:
resolved_sequence_lengths = get_active_sequence_lengths() + 1is correct in the prefill and MTP-decode steps —_rewind_kv_cachefolds accepted speculative tokens intokv_length_offsetsbefore the+1, matching the documented legacy ordering. Prefill rewind is a no-op and+1accounts for the newly sampled base token. - Accepted-token gating:
accepted_tokensgated onnum_decode_requests > 0is consistent withprepare_next_forward_passonly populating decode rows; theNonepath threads cleanly through the CPU-copy/resolve chain. - Grouped-token layout: base/draft writes validate MTP shape, transpose drafts correctly, and zero padding; dtype-safe.
- resolve_requests: hole-filling survivor order matches legacy
update_requests; token-row moves correctly skipped for the prefill→decode transition (prepare rebuilds them) withactive_token_count = 0. - EP alignment: primer issues dummy MTP-before-base forward to match steady-state collective ordering; dummy-step ordering covered by new tests.
Compliance checks: no new direct parallel_state.get_*_group() reads in megatron/core; changed-signature APIs have only internal/test callers (low external-compat risk); record_cg_wait probe path is non-mutating; no newly-introduced unused variables or arguments.
eb48d2a to
1091404
Compare
Signed-off-by: Lawrence McAfee <lmcafee@nvidia.com>
1091404 to
da72be8
Compare
Signed-off-by: Lawrence McAfee <lmcafee@nvidia.com>
589c6f1 to
1965c8d
Compare
Signed-off-by: Lawrence McAfee <lmcafee@nvidia.com>
Signed-off-by: Lawrence McAfee <lmcafee@nvidia.com>
Signed-off-by: Lawrence McAfee <lmcafee@nvidia.com>
Signed-off-by: Lawrence McAfee <lmcafee@nvidia.com>
Signed-off-by: Lawrence McAfee <lmcafee@nvidia.com>
Signed-off-by: Lawrence McAfee <lmcafee@nvidia.com>
|
@sidsingh-nvidia Thanks for the detailed review. The naming concern is addressed by renaming the schedules according to their actual execution property: I'd prefer to keep the phase orchestration in the controller. Sampling/MTP, pending-logit state, request resolution, events, and forward submission constitute one controller step. Splitting that step around engine admission would expose partially completed controller state across calls and make the ordering contract harder to preserve. The synchronous admission callback keeps engine-owned queue mutation at one explicit point without moving controller state into the engine. I agree that the two async dummy cases can share one controller primitive. I'll consolidate those using a The counter workaround mentioned at the end has also been removed in |
Signed-off-by: Lawrence McAfee <lmcafee@nvidia.com>
Signed-off-by: Lawrence McAfee <lmcafee@nvidia.com>
Signed-off-by: Lawrence McAfee <lmcafee@nvidia.com>
Signed-off-by: Lawrence McAfee <lmcafee@nvidia.com>
Signed-off-by: Lawrence McAfee <lmcafee@nvidia.com>
Signed-off-by: Lawrence McAfee <lmcafee@nvidia.com>
Signed-off-by: Lawrence McAfee <lmcafee@nvidia.com>
|
/ok to test 5b236ba |
Signed-off-by: Lawrence McAfee <lmcafee@nvidia.com>
Signed-off-by: Lawrence McAfee <lmcafee@nvidia.com>
|
/ok to test 2c957eb |
|
🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA/Megatron-LM/actions/runs/30374569144 |
Summary
Extend dynamic inference asynchronous scheduling across the remaining request and output features needed by the primary inference path. The controller keeps one pending forward while CPU request work is reordered around GPU sampling and forward execution.
Architecture
legacyandasync.Routing replay remains explicitly unsupported by this PR.