Repository navigation
Move in-model CP layout helpers into miles_plugins/models/cp_utils.py - #1814
Merged
Merged
Conversation
yueming-yuan
requested review from
Shi-Dong,
Zhichenzzz,
fzyzcjy,
maocheng23 and
yushengsu-thu
as code owners
July 27, 2026 04:48
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
yueming-yuan
force-pushed
the
yueming/cp-layout-utils-to-plugins
branch
from
July 27, 2026 15:54
bb12d56 to
f8e5059
Compare
yueming-yuan
force-pushed
the
yueming/cp-layout-utils-to-plugins
branch
from
September 3, 2026 03:29
55c3ac8 to
e2aa3a0
Compare
miles/backends/training_utils/cp_utils.py owns the CP helpers the training backend applies around the model: slicing data, masks, logprobs and logits. build_gdn_cp_context was the one exception -- it runs inside a layer forward, is used only by model plugins, and forced an optional flash-linear-attention dependency (the fla.ops.cp try/except) into a core training module. The zigzag <-> contiguous-shard relayout has the same shape and was private to miles_plugins/models/hf_attention.py, so only the two models built on HuggingfaceAttention could reach it. Collect both in miles_plugins/models/cp_utils.py, which now holds the CP helpers that run inside a forward. To break the resulting import cycle, detect_and_setup_hybrid_cp moves the other way, into hf_attention.py next to the HuggingfaceAttention class it is defined in terms of. Pure move: every relocated definition is byte-identical apart from dropping the underscore prefix on the four newly public relayout helpers.
yueming-yuan
force-pushed
the
yueming/cp-layout-utils-to-plugins
branch
from
September 4, 2026 00:52
e2aa3a0 to
256c4eb
Compare
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.
What
Pure refactor, no behavior change.
miles/backends/training_utils/cp_utils.pyowns the CP helpers the training backend applies around the model —slice_with_cp,natural_to_zigzag_slice,slice_loss_masks_for_local_cp,all_gather_with_cp,allgather_cp_redistribute,get_logits_and_tokens_offset_with_cp, … All of them operate on tokens / masks / logprobs / logits outsideforward(), and are called fromloss.py,data.py,losses.py.build_gdn_cp_contextwas the one exception. It runs inside a layer's forward, its only callers aremiles_plugins/models/qwen3_next.pyandqwen3_5.py, and it forced an optional flash-linear-attention dependency into a core training module:The zigzag ↔ contiguous-shard relayout has the same shape but the opposite problem: it lived as private helpers inside
miles_plugins/models/hf_attention.py, so only models built onHuggingfaceAttentioncould reach it. It is not HF-attention-specific — it bridges Megatron's zigzag CP layout to the contiguous rank-local shard every fla CP operator expects, and any model with an fla linear-attention layer needs it.Change
miles_plugins/models/cp_utils.pybecomes the module for CP helpers that run inside a forward:build_gdn_cp_context(moved from core) plus the relayout helperszigzag_to_packed_shard/packed_shard_to_zigzagand their internals (moved fromhf_attention.py, underscore prefix dropped).detect_and_setup_hybrid_cpmoves the other way, out ofmiles_plugins/models/cp_utils.pyintohf_attention.py. It is defined in terms ofisinstance(module, HuggingfaceAttention), so it belongs next to that class — and moving it is what breaks the import cyclehf_attention → cp_utils → hf_attention.cp_utils.pylosesbuild_gdn_cp_context, thefla.ops.cptry/except, and its now-unusedtorch.nnimport.qwen3_next.py,qwen3_5.py,miles/backends/megatron_utils/actor.py,tests/e2e/precision/test_hf_attention_cp_relayout.py.Verification
astand comparing source text (10/10 identical).pre-commit run --all-filesclean.miles_plugins.models.cp_utilsnow has zero intra-repo imports; corecp_utilsimports only.parallel.tests/e2e/precision/test_hf_attention_cp_relayout.pyround-trip test, which now imports the public names. It needs GPUs, so it runs in CI rather than locally.Motivation
New model support adds a third model that needs the relayout, and it is not a
HuggingfaceAttentionsubclass. Without this, it would either duplicate the permutation logic or import a private symbol across plugins. This PR stands on its own, though — it removes an optional-dependency leak from a core training module either way.