Skip to content

Tiny refactor branching in getting dataset samples - #27

Merged
fzyzcjy merged 3 commits into
mainfrom
feat/refactor_get_samples
Oct 12, 2025
Merged

Tiny refactor branching in getting dataset samples#27
fzyzcjy merged 3 commits into
mainfrom
feat/refactor_get_samples

Conversation

@fzyzcjy

@fzyzcjy fzyzcjy commented Oct 12, 2025

Copy link
Copy Markdown
Collaborator

No description provided.

@fzyzcjy fzyzcjy changed the title Tiny refactor branching in getting samples Tiny refactor branching in getting training samples Oct 12, 2025
@fzyzcjy fzyzcjy changed the title Tiny refactor branching in getting training samples Tiny refactor branching in getting dataset samples Oct 12, 2025
@fzyzcjy
fzyzcjy merged commit 991ccce into main Oct 12, 2025
1 check passed
fzyzcjy pushed a commit that referenced this pull request Mar 19, 2026
JD-ETH pushed a commit to JensenFire/miles that referenced this pull request Apr 11, 2026
GuanxingLu pushed a commit to GuanxingLu/miles that referenced this pull request Apr 21, 2026
yushengsu-thu added a commit that referenced this pull request Aug 13, 2026
… grad norm

Found by the fixed-bridge GPU re-validation (4xH200 GPT-OSS 20B expert-only
LoRA, TP=2+SP, EP=1/ETP=1, full recompute): every reported per-slot grad_norm
came out exactly sqrt(2)x the true gradient norm (diag: reduced norm
247.1967 vs local l2 174.7945, ratio 1.41421 on every optim of every slot),
and grad_clip_norm under-scaled by the same factor (post-clip l2 0.7071 for
clip=1.0).

Mechanism: the bridge's grouped-expert adapter weights carry
tensor_model_parallel=True unconditionally (upstream-ported attribute
stamping in radixark/Megatron-Bridge#27). The only supported multi-LoRA MoE
config is expert_tensor_parallel_size=1, where those weights are fully
TP-DUPLICATED whenever TP>1 — so Megatron's attribute-based TP-duplicate
filter admits every rank's identical gradient into the world-reduced norm and
over-counts each logical parameter TP times. This was unobservable before the
bridge fix only because rank1's expert-adapter gradients were identically
zero (the expert-DDP routing bug); once #27 made them real, the double-count
became real too.

Fix at the existing pre-wrap seam: after the LoRA transform, clear
tensor_model_parallel on grouped-expert adapter weights when TP >
expert-TP (the duplicated case), so the stock filter counts each logical
param once (TP rank 0) — semantically identical to run-E of the 0812 matrix,
which was GPU-verified with true norms and synced ranks. DDP expert-bucket
routing keys on 'allreduce' and is untouched; the hook runs pre-wrap so the
fp32 masters copy the corrected attribute at optimizer build. Genuinely
TP-sharded (attention) adapters keep their flag. CPU regression tests pin
the cleared/kept/no-op shapes.
yushengsu-thu added a commit that referenced this pull request Aug 13, 2026
bridge is now radixark/Megatron-Bridge's default branch and the deliberate
integration line for the multi-LoRA work (#27 and follow-ups land there), so
the image should follow it without a Dockerfile edit per merge. 005135d had
pinned the then-HEAD 688d34b85 by SHA; 76ac997 already returned
Dockerfile.rocm to the @bridge form — this aligns the cu13 Dockerfile.

The caveat the SHA pin protected against is now stated instead of engineered
around: buildkit caches the install layer on the instruction text alone, so a
rebuild only picks up new bridge commits with --no-cache (or an explicit
cache-bust) — a stale cache silently keeps the old revision.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant