Add tensor and context parallel inference to the core runtime - #619
jmccaffrey-nv wants to merge 13 commits into
Conversation
Signed-off-by: Jonathan McCaffrey <jmccaffrey@nvidia.com>
Signed-off-by: Jonathan McCaffrey <jmccaffrey@nvidia.com>
Signed-off-by: Jonathan McCaffrey <jmccaffrey@nvidia.com>
Signed-off-by: Jonathan McCaffrey <jmccaffrey@nvidia.com>
Signed-off-by: Jonathan McCaffrey <jmccaffrey@nvidia.com>
|
/ok to test 128732b |
|
Signed-off-by: Jonathan McCaffrey <jmccaffrey@nvidia.com>
Signed-off-by: Jonathan McCaffrey <jmccaffrey@nvidia.com>
|
/ok to test 3de71c5 |
|
Thanks for doing this, the overall approach makes sense to me. Just a few comments (mostly from me asking codex about this PR): P1 - Could you please make sure the process group created by init_parallel() is cleaned up? It looks like this path bypasses the existing atexit registration, and the v2 CLI never calls the distributed shutdown helper. Perhaps it could follow the shutdown pattern used by flashdreams-run. P2 - It looks like reusing an existing process group without LOCAL_RANK resets every rank to cuda:0, even if the caller already selected a device. Do we need to preserve the current CUDA device or accept an explicit device? P2 - It looks like request_new_session() is discarded during distributed execution, causing the application to exit instead of starting the replacement session. Is that intended behavior? P2 - It looks like distributed sessions skip the normal unfinished-UI check once inference completes. Should rank zero retain the existing post-inference UI lifecycle while the worker ranks wait for a coordinated exit? Up to you if you think the P2s are worth addressing. |
Signed-off-by: Jonathan McCaffrey <jmccaffrey@nvidia.com>
|
/ok to test 823746f |
Signed-off-by: Jonathan McCaffrey <jmccaffrey@nvidia.com>
|
/ok to test 1950038 |
Signed-off-by: Jonathan McCaffrey <jmccaffrey@nvidia.com>
|
/ok to test 2b29590 |
Signed-off-by: Jonathan McCaffrey <jmccaffrey@nvidia.com>
|
/ok to test 4c996ff |
Signed-off-by: Jonathan McCaffrey <jmccaffrey@nvidia.com>
|
/ok to test 066df61 |
Signed-off-by: Jonathan McCaffrey <jmccaffrey@nvidia.com>
|
/ok to test d4446e4 |
|
Thanks - I agreed these were worth addressing and pushed the fixes through
I also incorporated the follow-on distributed edge cases Greptile found (idle UI timeout, failure-path collectives, and deadline divergence). Final-head CPU, GPU, MP4, WorldLens, docs, REUSE, and OSRB checks all pass. Greptile is now 5/5 with no outstanding/actionable findings. |
Summary
Review follow-up
init_parallel(): successful v2 CLI runs use coordinated shutdown, failures avoid peer waits, and the owned-world exit fallback skips potentially blocking destruction after an uncaught rank failure.LOCAL_RANKis unavailable.Validation
ci_cpupytest): passed.git diff --check: passed.Release review
Reviewed for public release. No secrets, internal infrastructure references, or non-public benchmark results were found in the final GitHub diff.