Skip to content

fix(loader): gate CUTLASS grouped NVFP4 MoE on architecture, not model_type string#260

Open
rsafier wants to merge 1 commit into
mainfrom
fix/generic-nvfp4-moe-gate
Open

fix(loader): gate CUTLASS grouped NVFP4 MoE on architecture, not model_type string#260
rsafier wants to merge 1 commit into
mainfrom
fix/generic-nvfp4-moe-gate

Conversation

@rsafier

@rsafier rsafier commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

Draft until the runtime regression check lands (building now — validating Holo-3.1-35B and AgentWorld both engage CUTLASS grouped + stay coherent on the actual binary).

Problem

The fast CUTLASS grouped NVFP4 MoE path (opt-in via ATLAS_HOLO_LOW_MEMORY_MOE=1) was gated on config.model_type == "holo3_1_moe", so any other NVFP4-MoE checkpoint silently fell back to the slow moe_w4a16 path even with the flag set. Concrete repro: sakamakismile/Qwen-AgentWorld-35B-A3B-NVFP4 (model_type qwen3_5_moe, compressed-tensors NVFP4) — no CUTLASS grouped SFB signal, ran moe_w4a16.

But holo3_1_moe / qwen3_5_moe / qwen3_6_moe are the same architecture at the weight levelfactory::loader_for_config maps all three to Qwen35WeightLoader, and holo3_1_moe is a label fabricated at config/dispatch.rs:120-128 (rewrites qwen3_5_moeholo3_1_moe when image_token_id==248056) purely so kernel-target directory lookup can disambiguate 3.5 vs 3.6 (they collide on hidden=2048). Downstream code then mistook that label for an architecture discriminator.

Fix (2 gates, one file — qwen35/load_layers.rs)

  • holo_nvfp4_moenvfp4_moe = config.num_experts > 0 && quant_format == Nvfp4 (architecture, not label).
  • is_holo_modelopt_mixed_precisionis_modelopt_mixed_precision: drop the model_type == "holo3_1_moe" conjunct; keep the quant-config test (any Qwen3.5/3.6-family MoE shipping modelopt MIXED_PRECISION is now recognized).

Scope / safety

  • No kernels move. The config/dispatch.rs model_type rewrites (needed for kernel-directory resolution) are deliberately untouched — this only changes which MoE dispatch fires, not kernel layout.
  • No regression on Holo: holo3_1_moe still qualifies (num_experts=256, Nvfp4), and the mixed-precision path still fires for Holo (modelopt MIXED_PRECISION). It now also fires for architecturally-identical siblings shipping the same quant format — no existing checkpoint changes behavior (we ship no non-Holo modelopt-MIXED MoE).
  • This is the focused bug fix. A broader capability-based refactor (replacing model_type string checks across the codebase — MTP allowlist, MLA guard, etc.) is a separate, larger effort scoped for a follow-up.

@rsafier

rsafier commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

Runtime regression — PASS (validated on the actual binary, both models)

Built spark-genmoe (main + this fix, ATLAS_TARGET_MODEL=*), served each single-node with ATLAS_HOLO_LOW_MEMORY_MOE=1:

AgentWorld (qwen3_5_moe, compressed-tensors NVFP4) — the fix:

  • CUTLASS grouped SFB: built 256 experts ×40 layers (was falling back to moe_w4a16 before)
  • Coherent: "The capital of France is Paris."

Holo-3.1-35B-A3B-NVFP4 (holo3_1_moe, modelopt MIXED_PRECISION) — no regression:

  • CUTLASS grouped SFB ×40 layers (unchanged)
  • is_modelopt_mixed_precision still fires: "routing Holo ModelOpt SSM/attention projections through native FP8", "Layer N: SSM native FP8 — w8a16" — the modelopt path is byte-for-byte the same
  • Coherent: "15 plus 27 equals 42."

No kernels moved; config/dispatch.rs rewrites untouched. cargo check + fmt clean.

@rsafier rsafier marked this pull request as ready for review July 5, 2026 03:41
rsafier added a commit to MonumentalSystems/atlas that referenced this pull request Jul 8, 2026
…ot model_type

check_kernel_batched_eligible() blocked batched-prefill attention for MLA models
via `config.model_type == "mistral"` — a hardcoded proxy the author flagged as
conservative ("mistral is the only MLA model in Atlas today"). Key it on the
actual MLA capability instead: `config.kv_lora_rank > 0` (== AttentionType::Mla).

- signature: `model_type: &str` -> `is_mla: bool`; wrapper passes
  `self.config.kv_lora_rank > 0` (the exact derivation of
  capabilities().attention_type == Mla; read directly to avoid rebuilding the
  full ModelCapabilities struct on this per-prefill-batch hot path).
- tests updated to pass the bool.

Behavior-preserving on every shipped model: kv_lora_rank>0 == {mistral,
deepseek_v4}; all other model_types have kv_lora_rank=0 -> is_mla=false ->
unchanged. deepseek_v4 is the only newly-matched model, but it is ALREADY
rejected one check later by `head_dim > 256` (head_dim=512), so its outcome
(not batched-eligible) is identical. Also future-proofs any MLA checkpoint
shipped with head_dim<=256. Unit tests (batch_kernel_tests) pass.

Stacked on Avarok-Cybersecurity#260 (fix/generic-nvfp4-moe-gate). NOTE: an exhaustive adversarial
pass over the OTHER model_type gates found this is the ONLY additional safe
architecture-generic conversion — notably the MTP allowlist (preflight.rs) must
NOT be converted to has_mtp (broader than intent: would re-enable the
MiniMax/DeepSeek per-module-MTP path the guard rejects). See PR description.
SeedSource pushed a commit that referenced this pull request Jul 9, 2026
…ot model_type (#261)

check_kernel_batched_eligible() blocked batched-prefill attention for MLA models
via `config.model_type == "mistral"` — a hardcoded proxy the author flagged as
conservative ("mistral is the only MLA model in Atlas today"). Key it on the
actual MLA capability instead: `config.kv_lora_rank > 0` (== AttentionType::Mla).

- signature: `model_type: &str` -> `is_mla: bool`; wrapper passes
  `self.config.kv_lora_rank > 0` (the exact derivation of
  capabilities().attention_type == Mla; read directly to avoid rebuilding the
  full ModelCapabilities struct on this per-prefill-batch hot path).
- tests updated to pass the bool.

Behavior-preserving on every shipped model: kv_lora_rank>0 == {mistral,
deepseek_v4}; all other model_types have kv_lora_rank=0 -> is_mla=false ->
unchanged. deepseek_v4 is the only newly-matched model, but it is ALREADY
rejected one check later by `head_dim > 256` (head_dim=512), so its outcome
(not batched-eligible) is identical. Also future-proofs any MLA checkpoint
shipped with head_dim<=256. Unit tests (batch_kernel_tests) pass.

Stacked on #260 (fix/generic-nvfp4-moe-gate). NOTE: an exhaustive adversarial
pass over the OTHER model_type gates found this is the ONLY additional safe
architecture-generic conversion — notably the MTP allowlist (preflight.rs) must
NOT be converted to has_mtp (broader than intent: would re-enable the
MiniMax/DeepSeek per-module-MTP path the guard rejects). See PR description.
@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: Broadens NVFP4 MoE and modelopt mixed-precision detection, removing model_type gates to support more checkpoints.
  • Serving-path files: crates/spark-model/src/weight_loader/qwen35/load_layers.rs

Serve gate

  • Build: 18s - Qwen3.6-35B-A3B-FP8 on GB10 node1
  • Top-k KL vs main: 0.0001 (no shift vs main) - decode delta -0.0%
  • Coherence probe: coherence=pass ping=10/10 (note: our test_coherence.py is flaky here; KL is the reliable serve signal)

GLM-5.2 code review

Correctness: Sound. Gating on num_experts > 0 + QuantFormat::Nvfp4 is the right architectural check; removing the model_type == "holo3_1_moe" string match correctly unblocks sibling/third-party checkpoints routed through Qwen35WeightLoader. The is_modelopt_mixed_precision rename and de-gating is consistent — the quant_config fields are sufficient and the model_type check was redundant exclusion.

Risk: Moderate. The broadened nvfp4_moe predicate now activates the CUTLASS grouped NVFP4 MoE transpose path for any NVFP4 MoE checkpoint. If a non-Holo NVFP4 MoE has expert weight layouts that differ from what the grouped path expects (e.g., different packing, scaling block size), this will silently route through an untested path rather than the known-safe moe_w4a16 fallback. The env-var gate (ATLAS_HOLO_LOW_MEMORY_MOE=1) provides a mitigation, but only if operators know to set it.

Quality: Comment updates are accurate and well-scoped. eq_ignore_ascii_case("MIXED_PRECISION") is pre-existing and technically wrong (ASCII-case-insensitive compare against an uppercase literal matches lowercase but not mixed-case like "Mixed_Precision") — not introduced here, but worth a follow-up.

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 fix/generic-nvfp4-moe-gate branch from 650e5ce to cb79d49 Compare July 10, 2026 00:39
rsafier added a commit that referenced this pull request Jul 10, 2026
…ot model_type

check_kernel_batched_eligible() blocked batched-prefill attention for MLA models
via `config.model_type == "mistral"` — a hardcoded proxy the author flagged as
conservative ("mistral is the only MLA model in Atlas today"). Key it on the
actual MLA capability instead: `config.kv_lora_rank > 0` (== AttentionType::Mla).

- signature: `model_type: &str` -> `is_mla: bool`; wrapper passes
  `self.config.kv_lora_rank > 0` (the exact derivation of
  capabilities().attention_type == Mla; read directly to avoid rebuilding the
  full ModelCapabilities struct on this per-prefill-batch hot path).
- tests updated to pass the bool.

Behavior-preserving on every shipped model: kv_lora_rank>0 == {mistral,
deepseek_v4}; all other model_types have kv_lora_rank=0 -> is_mla=false ->
unchanged. deepseek_v4 is the only newly-matched model, but it is ALREADY
rejected one check later by `head_dim > 256` (head_dim=512), so its outcome
(not batched-eligible) is identical. Also future-proofs any MLA checkpoint
shipped with head_dim<=256. Unit tests (batch_kernel_tests) pass.

Stacked on #260 (fix/generic-nvfp4-moe-gate). NOTE: an exhaustive adversarial
pass over the OTHER model_type gates found this is the ONLY additional safe
architecture-generic conversion — notably the MTP allowlist (preflight.rs) must
NOT be converted to has_mtp (broader than intent: would re-enable the
MiniMax/DeepSeek per-module-MTP path the guard rejects). See PR description.
@rsafier rsafier force-pushed the fix/generic-nvfp4-moe-gate branch from cb79d49 to fdc6bba Compare July 12, 2026 22:46
rsafier added a commit that referenced this pull request Jul 12, 2026
…ot model_type

check_kernel_batched_eligible() blocked batched-prefill attention for MLA models
via `config.model_type == "mistral"` — a hardcoded proxy the author flagged as
conservative ("mistral is the only MLA model in Atlas today"). Key it on the
actual MLA capability instead: `config.kv_lora_rank > 0` (== AttentionType::Mla).

- signature: `model_type: &str` -> `is_mla: bool`; wrapper passes
  `self.config.kv_lora_rank > 0` (the exact derivation of
  capabilities().attention_type == Mla; read directly to avoid rebuilding the
  full ModelCapabilities struct on this per-prefill-batch hot path).
- tests updated to pass the bool.

Behavior-preserving on every shipped model: kv_lora_rank>0 == {mistral,
deepseek_v4}; all other model_types have kv_lora_rank=0 -> is_mla=false ->
unchanged. deepseek_v4 is the only newly-matched model, but it is ALREADY
rejected one check later by `head_dim > 256` (head_dim=512), so its outcome
(not batched-eligible) is identical. Also future-proofs any MLA checkpoint
shipped with head_dim<=256. Unit tests (batch_kernel_tests) pass.

Stacked on #260 (fix/generic-nvfp4-moe-gate). NOTE: an exhaustive adversarial
pass over the OTHER model_type gates found this is the ONLY additional safe
architecture-generic conversion — notably the MTP allowlist (preflight.rs) must
NOT be converted to has_mtp (broader than intent: would re-enable the
MiniMax/DeepSeek per-module-MTP path the guard rejects). See PR description.
…l_type string

The fast CUTLASS grouped NVFP4 MoE path (opt-in via ATLAS_HOLO_LOW_MEMORY_MOE=1)
was gated on `config.model_type == "holo3_1_moe"`, silently dropping every other
NVFP4-MoE checkpoint to the slow `moe_w4a16` path. But holo3_1_moe / qwen3_5_moe /
qwen3_6_moe are the SAME architecture at the weight level (factory::
loader_for_config maps all three to Qwen35WeightLoader); "holo3_1_moe" is a label
fabricated at config/dispatch.rs purely for kernel-target directory lookup. The
grouped path operates on the loaded NVFP4 experts regardless of source quant
format (the inline comment already said so).

Repro: Qwen-AgentWorld-35B-A3B-NVFP4 (model_type qwen3_5_moe, compressed-tensors
NVFP4) fell back to moe_w4a16 even with LOW_MEMORY_MOE=1.

Fix — gate on the architecture (MoE + NVFP4), not the label:
- holo_nvfp4_moe  -> nvfp4_moe = `config.num_experts > 0 && quant_format == Nvfp4`
- is_holo_modelopt_mixed_precision -> is_modelopt_mixed_precision: drop the
  `model_type == "holo3_1_moe"` conjunct, keep the quant-config test.

No kernels move; the config/dispatch.rs model_type rewrites (kernel-dir
resolution) are untouched. holo3_1_moe still qualifies (num_experts=256, Nvfp4)
so Holo-3.1-35B is unchanged; the mixed-precision path still fires for Holo and
now also for any architecturally-identical sibling shipping the same quant.
@rsafier rsafier force-pushed the fix/generic-nvfp4-moe-gate branch from fdc6bba to 6a48210 Compare July 13, 2026 00:43
rsafier added a commit that referenced this pull request Jul 13, 2026
…ot model_type

check_kernel_batched_eligible() blocked batched-prefill attention for MLA models
via `config.model_type == "mistral"` — a hardcoded proxy the author flagged as
conservative ("mistral is the only MLA model in Atlas today"). Key it on the
actual MLA capability instead: `config.kv_lora_rank > 0` (== AttentionType::Mla).

- signature: `model_type: &str` -> `is_mla: bool`; wrapper passes
  `self.config.kv_lora_rank > 0` (the exact derivation of
  capabilities().attention_type == Mla; read directly to avoid rebuilding the
  full ModelCapabilities struct on this per-prefill-batch hot path).
- tests updated to pass the bool.

Behavior-preserving on every shipped model: kv_lora_rank>0 == {mistral,
deepseek_v4}; all other model_types have kv_lora_rank=0 -> is_mla=false ->
unchanged. deepseek_v4 is the only newly-matched model, but it is ALREADY
rejected one check later by `head_dim > 256` (head_dim=512), so its outcome
(not batched-eligible) is identical. Also future-proofs any MLA checkpoint
shipped with head_dim<=256. Unit tests (batch_kernel_tests) pass.

Stacked on #260 (fix/generic-nvfp4-moe-gate). NOTE: an exhaustive adversarial
pass over the OTHER model_type gates found this is the ONLY additional safe
architecture-generic conversion — notably the MTP allowlist (preflight.rs) must
NOT be converted to has_mtp (broader than intent: would re-enable the
MiniMax/DeepSeek per-module-MTP path the guard rejects). See PR description.
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