Skip to content

feat(gdn-tp): dense-model GDN/SSM TP-shard (dense pure-TP) [stacks on #254+#250]#284

Open
rsafier wants to merge 15 commits into
mainfrom
feat/gdn-dense-tp-pr
Open

feat(gdn-tp): dense-model GDN/SSM TP-shard (dense pure-TP) [stacks on #254+#250]#284
rsafier wants to merge 15 commits into
mainfrom
feat/gdn-dense-tp-pr

Conversation

@rsafier

@rsafier rsafier commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Re-opened replacement for #259. #259 was merged into its base feature branch by mistake on 2026-07-09 and reverted; the base branch has since been restored and restacked onto main. GitHub cannot reopen a merged PR, so this is a fresh PR carrying the identical commits (verified patch-identical via git range-diff). Marked ready for review — the original's "draft" wording was stale.

Stacks on #254 + #250 (both unmerged). Base is #254 (feat/gdn-headparallel-tp-on-main); this branch also carries #250 (8ce0a600, native-BF16 dense loader) as its first commit because the dense-TP shard sits on #250s restructured dense SSM arm. The net-new commit is the tip (feat(gdn-tp): TP-shard the dense-model GDN/SSM heads). Once #250 + #254 land on main, rebase collapses this to just that commit.

What

Makes dense Holo models (0.8B/4B/9B) TP-shard under pure tensor-parallel. #254 gave the SSM-hybrid MoE models GDN HeadParallel; the dense loader was never wired for it, so dense TP2 produced garbage (TP1 fine). This wires the same shard_gdn_* slicers (tp_shard/gdn.rs) into the dense loaders SSM arm.

Loader-only. config holds per-rank-LOCAL linear head counts (topology.rs divides them), so TpGdnDims::from_config rebuilds the FULL pre-shard sizes; load/concat/interleave run at FULL, then shard_gdn_qkvz_rows / _ba_rows / _conv_rows / _value_vector (a_log, dt_bias) / _out_proj_row_parallel cut this ranks contiguous head range. norm.weight [vd] is replicated (shared across value heads). The post-out_proj all-reduce already fires in the shared Qwen3SsmLayer forward. Gated tp_size>1; byte-identical at tp==1 (else-arm pass-through, every slicer no-ops).

Validation — 2-node pure-TP2 (GB10 × 2)

Model Before After
dense 0.8B garbage (multilingual) coherent — "red, blue, and yellow"
dense 4B garbage coherent — "Paris"
MoE 35B (regression) unaffected — "15 plus 27 equals 42"

Rebuilt the branch binary (not the wip prototype) and re-confirmed 0.8B TP2 coherent on this exact base. Compiles + fmt clean.

Scope

BF16/NVFP4 dense only. The native-FP8 GDN SSM path (ATLAS_NO_GDN_FP8, gated on proj_is_fp8_any_scale) is not yet sharded — dense-FP8-TP is a follow-up (the BF16 dense models dont take that path). Mirrors #254 being MoE-TP-scoped.

@rsafier rsafier marked this pull request as ready for review July 10, 2026 00:48
@rsafier rsafier changed the title feat(gdn-tp): dense-model GDN/SSM TP-shard (dense pure-TP) [draft, stacks on #254+#250] feat(gdn-tp): dense-model GDN/SSM TP-shard (dense pure-TP) [stacks on #254+#250] Jul 10, 2026
@SeedSource

Copy link
Copy Markdown
Collaborator

[BETA] Automated PR review — SeedSource AI pipeline

Advisory only, not a merge gate. AI-generated (internal risk-triage, Atlas serve gate, GLM-5.2 review), attributed per Atlas convention.

Triage

  • Risk: med - serve-test-required (triage advisory): True - path-heuristic (authority): True
  • Rationale: Fixes BF16 snapshot and dispatch logic for FP8/NVFP4 builds to prevent memory issues and fallbacks.
  • Serving-path files: crates/spark-model/src/layers/qwen3_ssm/trait_decode_multi_seq/ssm_batched.rs, crates/spark-model/src/weight_loader/qwen35_dense.rs

Serve gate

  • Build: Nones - Qwen3.6-35B-A3B-FP8 on GB10 node1
  • Top-k KL vs main: None - decode delta None%
  • Coherence probe: None (note: our test_coherence.py is flaky here; KL is the reliable serve signal)

GLM-5.2 code review

Correctness concerns:

  1. ssm_batched.rs gate logic: The new gate (qkvz_fp8w.is_some() && w8a16_gemm_k.0 != 0) || (qkvz_fp8w.is_none() && dense_gemm_k.0 != 0) — if both qkvz_fp8w and qkvz_nvfp4 are None but dense_gemm_k.0 == 0, qkvz_ok is false and falls to per-seq loop. Verify dense_gemm_k is always loaded in the BF16 build; otherwise this is a silent perf regression.

  2. ffn_bf16_snapshot D2D before load_dense_ffn: The comment claims dense_auto returns the store's cached BF16 ptr, and load_dense_ffn's quantized_any frees it. If dense_auto returns a copy (not the cached ptr), the snapshot is unnecessary but harmless; if it returns the same ptr, the snapshot must happen before any free. Ordering looks correct, but confirm clone_bf16 reads happen before load_dense_ffn — they do (snapshot is built before the call). Risk: if dense_auto doesn't cache or the free path differs, the snapshot is dead code masking a real UAF.

  3. sharded_ptr != src.weight free: In load_bf16_dense, after shard_dense_bf16, if tp_size == 1 returns src directly (no free). For tp_size > 1, if shard_dense_bf16 returns the same ptr (no-op shard), the guard prevents double-free. OK.

  4. o_proj: QuantizedWeight::null(): Bf16Raw sets o_proj null, then set_o_dense_bf16 installs BF16. Verify decode/prefill paths check o_dense_bf16.is_some() before falling back to NVFP4 o_proj — a null NVFP4 o_proj with no BF16 override would crash.

  5. TpGdnDims::from_config: Rebuilds full sizes from per-rank-local config. If tp_size doesn't evenly divide head counts, full_conv_dim() = local * tp_size is wrong. Confirm topology.rs guarantees even division.

Quality: Comment density is high but mostly justified for the subtle memory-lifetime issues. The clone_bf16 closure captures lp, inter, h — verify h is the correct column dim for gate/up and row dim for down (looks right: gate/up are [inter, h], down is [h, inter]).

Risk: Medium. The FFN snapshot logic is the riskiest part — a wrong assumption about dense_auto/quantized_any interaction either wastes memory or reintroduces CUDA-700. Needs a test confirming Bf16Raw + prefill doesn't crash and that snapshot ptrs differ from store ptrs.

Pipeline: build+serve on GB10, GLM-5.2 via NVIDIA NIM, internal risk-triage classifier. External diff treated as untrusted input. Public repo.

@rsafier rsafier force-pushed the feat/gdn-headparallel-tp-on-main branch from d037098 to ea468ef Compare July 12, 2026 23:23
rsafier and others added 12 commits July 12, 2026 20:43
…rs + post-out_proj all-reduce (cargo check green)
…h decode (ATLAS_GDN_DECODE_GRAPH=1)

Collectives were already async (stream-ordered via all_reduce_async). Two speedups:
- register buffers.norm_output() with NCCL so the 16 attention o_proj all-reduces/token use cached IB MR + user-buffer zero-copy (was unregistered).
- ATLAS_GDN_DECODE_GRAPH=1 extends the existing decode CUDA-graph gate (was self.comm.is_none() only) to capture the full tp>1 decode forward (~130 kernels + 64 all-reduces) into one replayable graph; eager fallback on capture failure. Default off; tp=1 byte-identical.
… shard debug logs

The GDN gated-RMSNorm gain is over the value HEAD-DIM ([vd]=128), shared
across all value heads — NOT per-head [nv*vd]. The HeadParallel loader sharded
it on the value-head axis via shard_gdn_value_vector(unit=vd), reading
local_nv*vd*2 = 4096 bytes from the [vd] (~256-512B) buffer -> cuMemcpyDtoDAsync
INVALID_VALUE at model build (both loader sites: FP8-dense + NVFP4). Fix:
replicate norm.weight (every rank keeps the full [vd]); a_log/dt_bias stay
sharded (they ARE per-head [nv]). tp=1 unaffected (slicer was a no-op there).

Also add permanent tracing::debug! (target spark_model::tp_shard / ::concat) to
slice_segments, shard_gdn_value_vector, dense row-parallel, and gpu_concat_rows
so TP shard sizing is inspectable via RUST_LOG=spark_model::tp_shard=debug
instead of ad-hoc eprintln next time.
…ent + graph contiguity

The active decode list arrives in reverse-arrival order ([7,6,..,0] for 8
seqs), so batch position i mapped to SSM pool slot (n-1-i). Both the
batched-recurrent SSM (ssm_batched_recurrent.rs contiguity check:
states[i]==base+i*stride) and the multi-seq CUDA-graph capture (slot==i
assumption) then FAIL → fall back to the eager per-seq serial loop → no
concurrency scaling. The pool assigns consecutive slots {0..n-1}; sorting the
active list ascending by SSM slot (SequenceState::ssm_slot_idx, new pub
accessor) makes position i ↔ slot i, satisfying the contiguity invariant so
the batched paths engage. Reordering the whole ActiveSeq keeps the
position->seq logits mapping consistent; safe for all decode paths.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AoQXbv8a4aVXFPqj8nQqYN
(cherry picked from commit 0b5cf60)
…ruption)

The batched-projection SSM recurrent decode launched conv1d_update_l2norm
once with batch=n on the base deinterleaved pointer. The conv kernel strides
its input by dim(=conv_dim=8192), but deinterleaved is the QKVZ projection
output laid out [Q|K|V|Z] with stride qkvz_size(=12288). So seq b>=1 read from
b*conv_dim instead of b*qkvz_size, pulling the previous seq's Z-gate region
into the GDN scan: correct at n=1, corrupt at n>=2 (measured 0/3 at n=4).

Mirror the proven per-token pattern in trait_decode_batched_conv_gdn.rs: run
the cheap per-channel conv per-seq (batch=1) with pre-offset pointers so the
qkvz_size input stride and conv_dim output stride are both honored. The
expensive GDN scan stays fully batched. Diagnosis via subagent root-cause read.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AoQXbv8a4aVXFPqj8nQqYN
(cherry picked from commit 632e986)
- Split the GDN HeadParallel slicers out of tp_shard.rs into tp_shard/gdn.rs
  (704 -> 343 LoC; both under the 500 cap) — drops tp_shard.rs from the
  file-size allow-list; allow-list linear_attn_arms.rs (548) + trait_decode_batched.rs (503).
- clippy: repeat().take() -> std::iter::repeat_n() in tp_shard/tests.rs.
- typos: alog_src_off -> a_log_src_off.
- cargo fmt.
After splitting the GDN slicers into tp_shard/gdn.rs, the unit tests (a
sibling module) could no longer reach segment_copy_plan / CopyOp / the
qkv[z]_segments methods (were private, previously visible via super::* when
co-located). Mark them pub(crate) + re-export CopyOp/segment_copy_plan from
tp_shard so tests resolve them via super::*.
…port

The pub(crate) re-export in tp_shard.rs was unused in the lib (non-test)
build (deny(unused) -> error). Import CopyOp/segment_copy_plan directly in
tp_shard/tests.rs from the gdn submodule instead.
slot_idx is already usize on this branch — the `as usize` from the original
cherry-pick trips clippy::unnecessary_cast under deny(clippy::all).
Under pure-TP (--tp-size 2 --ep-size 1: world_size=2, ep_world_size=1) the
MoE experts are REPLICATED (local_expert_range returns all experts when
ep<=1), so every rank computes the full identical MoE output. But the
all-reduce + shared-expert-exclusion gated on comm.world_size()>1, summing
two full outputs -> 2x per MoE layer -> deterministic garbage from the first
token. Gate on config.ep_world_size>1 (the true 'experts are sharded'
invariant) at all 15 sites across 9 forward variants. Single-node/EP/EP+TP
unchanged; only pure-TP flips from wrongly-reducing to correctly-not.
@rsafier rsafier force-pushed the feat/gdn-headparallel-tp-on-main branch from ea468ef to f146240 Compare July 13, 2026 00:43
rsafier added 2 commits July 12, 2026 20:56
- Split the GDN HeadParallel slicers out of tp_shard.rs into tp_shard/gdn.rs
  (704 -> 343 LoC; both under the 500 cap) — drops tp_shard.rs from the
  file-size allow-list; allow-list linear_attn_arms.rs (548) + trait_decode_batched.rs (503).
- clippy: repeat().take() -> std::iter::repeat_n() in tp_shard/tests.rs.
- typos: alog_src_off -> a_log_src_off.
- cargo fmt.
…eckpoints

Load Hcompany Holo-3.1-0.8B/4B/9B (and Ornith dense) at native BF16
instead of the lossy runtime BF16->NVFP4 requant the no-metadata
(Bf16Raw) dense path defaulted to. Adds BF16 arms for the FFN,
attention (Q/K/V/O) and the SSM in_proj_qkvz/out_proj in qwen35_dense.rs,
and gates the batched-decode qkvz path (ssm_batched.rs) so the BF16
build engages the batched dense_gemm instead of dropping to the per-seq
loop. The NVFP4 weights stay installed as the spec-decode/batched fallback.

Fixes a use-after-free that crashed first-layer prefill with CUDA-700:
the FFN overlay installed gate/up/down via dense_auto(), which returns
the WeightStore cached BF16 ptr un-copied, but load_dense_ffn Bf16Raw
arm had already gpu.free() that buffer while building the NVFP4 fallback.
Snapshot fresh D2D copies before the free.

Validated on Holo-3.1-4B: coherent across FI prefill on/off and long
multi-chunk prefill, zero CUDA-700. With --lm-head-dtype bf16 the
agentic multi-turn suite goes 2/3 -> 3/3 vs the NVFP4 head at ~10%
single-stream decode cost (27.8 -> 25.1 tok/s); lossless head is worth
it for these small models (runtime-NVFP4 default was 72% agentic).
@rsafier rsafier force-pushed the feat/gdn-dense-tp-pr branch from d141e16 to e2e3521 Compare July 13, 2026 01:11
Wire the existing GDN HeadParallel slicers (tp_shard/gdn.rs, already used
by the MoE loader) into the dense loader SSM arm so dense Holo models
(0.8B/4B/9B) TP-shard under pure tensor-parallel. Before this the dense
SSM arm loaded the GDN weights UNSHARDED on every rank while the forward
ran HeadParallel all-reduces -> garbage under tp>1 (tp=1 was fine).

Loader-only. config holds per-rank-LOCAL linear head counts (topology.rs
divides them), so TpGdnDims rebuilds the FULL pre-shard sizes; load/
concat/interleave run at FULL, then shard_gdn_{qkvz_rows,ba_rows,
conv_rows,value_vector,out_proj_row_parallel} cut this rank contiguous
head range. norm.weight [vd] is REPLICATED (shared across value heads).
The post-out_proj all-reduce already fires in the shared Qwen3SsmLayer
forward. Byte-identical at tp_size==1 (gated else pass-through).

Validated 2-node pure-TP2 (GB10x2): dense 0.8B + 4B coherent (were
garbage before), MoE 35B unaffected ("15 plus 27 equals 42"). Scope:
BF16/NVFP4 dense; the native-FP8 GDN SSM path (ATLAS_NO_GDN_FP8) is not
yet sharded (follow-up). Stacks on #250 + #254.
@rsafier rsafier force-pushed the feat/gdn-dense-tp-pr branch from e2e3521 to eb540d4 Compare July 13, 2026 01:21
Base automatically changed from feat/gdn-headparallel-tp-on-main to main July 13, 2026 04:20
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.

2 participants