[cherry-pick][release/2.13] MD-TRT: move USE_C10D_NCCL from copts to defines (#4358 BUILD)#4424
Merged
Merged
Conversation
…chtrtrun The example used the multinode form (--nnodes/--node_rank/--rdzv_endpoint), which is only for multinode. Use the single-node 2-GPU invocation instead. Co-Authored-By: Claude Opus 4.8 (1M context) <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
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.
Corrected cherry-pick of the
core/runtime/BUILDchange from #4358 onto currentrelease/2.13, replacing the earlier PR #4374 which dropped it.What this fixes
The BUILD portion of #4358 moves the NCCL macro from a target-local
coptsflag to a propagatingdefines:This matters because
coptsapplies-DUSE_C10D_NCCLonly toruntime_base's own translation units, whereasdefinespropagatesUSE_C10D_NCCLto every dependent that compiles against the runtime headers — keeping the#ifdef USE_C10D_NCCLcode paths consistent across the dependency graph.Why a new PR
The original cherry-pick was resolved when
release/2.13predated the IRuntimeConfig infrastructure, and the BUILD change was dropped entirely.release/2.13has since addedRuntimeSettings.*/TRTRuntimeConfig.*and thedefines = select({TRT_HAS_IRUNTIME_CONFIG})block, so #4358's BUILD change now applies cleanly on top of it. This branch is rebuilt on currentrelease/2.13; the only delta is thecopts→definesmerge —RuntimeSettings/TRTRuntimeConfigare retained, nothing dropped.The other two files from #4358 (
_utils.py,_TRTInterpreter.py) are already present onrelease/2.13, so they produce no delta here.Supersedes #4374. Original PR: #4358
🤖 Generated with Claude Code