Skip to content

Add opt-in --paged-attention export for dense MLA (LATENT PagedAttention) [Slice 3B] - #599

Merged
justinchuby merged 3 commits into
mainfrom
squad/mobius-3b-paged-attention-export
Aug 25, 2026
Merged

Add opt-in --paged-attention export for dense MLA (LATENT PagedAttention) [Slice 3B]#599
justinchuby merged 3 commits into
mainfrom
squad/mobius-3b-paged-attention-export

Conversation

@justinchuby

Copy link
Copy Markdown
Member

Slice 3B — Mobius opt-in --paged-attention export (dense MLA, LATENT)

Emits com.microsoft::PagedAttention v1 in LATENT / absorbed-MLA mode for property-compatible dense MLA (DeepSeek-V2/V3, GLM-5.2 --glm-full-attention). Builds on the merged onnx-genai native runtime contract: audit/typed-validator/oracle (#1940), KV index emission (#1955), CUDA LATENT kernel (#1978).

Guarantees

  • Default off. Feature-off exports are byte-identical to the current dense-MLA graph (all new code is gated behind the flag/task). A byte-identical test locks this.
  • Property-based eligibility, never model names. Decided from semantic geometry: latent width, v_head_size, partial-RoPE suffix (rotary_offset = kv_lora_rank), cache dtype (fp16/bf16), and the onnx-genai-kv page/block constraints (head_size %8, kv_lora %8, rotary_dim %16, rotary_offset %8, block pow2 ≥16).
  • Typed reject, never silent fallback. Active query-dependent sparse selection (GLM DSA/IndexShare, DeepSeek-V4 CSA/HCA), MTP, quantized cache, head_sink, qk-norm, sliding window all raise a typed reason at model construction / build. Feature-on with an incompatible geometry errors. GLM's vestigial indexer config (dropped under --glm-full-attention) does not reject.
  • One cache authority. The op consumes/mutates caller-owned page buffers in place; Mobius allocates no pages and creates no second cache manager. onnx-genai-kv stays the sole authority. The graph binds caller-owned block_table / slot_mapping / cumulative + past lengths / per-layer LATENT key_cache, aliases the cache in place, and derives token positions from the length tensors (no position_ids input).

Emitted contract

  • One PagedAttention node per layer, _domain="com.microsoft", LATENT attrs (kv_cache_layout=LATENT, kv_num_heads=1, explicit scale, v_head_size=kv_lora_rank, rotary_offset=kv_lora_rank, do_rotary=1); rotary_dim is derived from cos_cache, never emitted.
  • Model inputs: input_ids, block_table (i32), slot_mapping (i32), cumulative_sequence_length (i32), past_seqlens (i32), per-layer key_cache.{i} [num_blocks, block_size, 1, head_size]. Outputs: logits, updated_key_cache.{i} (aliases input).
  • kv_b_proj is absorbed into the query/output projections at weight-apply time; numeric contract mirrors the onnx-genai equivalence oracle.

Tests (same commit)

  • component (components/_paged_mla_test.py): eligibility rejects, absorption, numpy decomposed-vs-absorbed LATENT parity (rel < 1e-6), structural emission.
  • model-level (models/paged_mla_export_test.py): feature-off byte-identical; feature-on structural + exact op attrs/inputs/outputs/model IO; typed rejects (DSA/CSA/HCA/MTP/window); torch-tensor weight absorption; DeepSeek-V3 path proves eligibility is not name-gated.
  • CLI (tests/cli_test.py): --features paged-attention plumbing + task resolution + mutual-exclusion errors.
  • Existing GLM / DeepSeek / task / build-graph tests pass unchanged.

Scope / gates

Draft — do not merge. Independent review required (reviewer excluding Leon/Sapper; Gaff or Roy final approval).

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

…ion)

Slice 3B: emit `com.microsoft::PagedAttention` v1 in LATENT/absorbed-MLA
mode for property-compatible *dense* MLA (DeepSeek-V2/V3, GLM-5.2
`--glm-full-attention`). Feature is default off; feature-off exports are
byte-identical to the current dense-MLA graph.

Eligibility is decided purely from semantic geometry (latent width,
v_head_size, partial-RoPE suffix, cache dtype, page/block constraints),
never from model names. Active query-dependent sparse selection (GLM DSA,
DeepSeek-V4 CSA/HCA), MTP, quantized cache, head_sink, qk-norm and
sliding window are typed-rejected at construction/build; feature-on with
an incompatible geometry errors rather than silently falling back to
dense. GLM's vestigial indexer config (dropped under --glm-full-attention)
does not reject.

The op consumes/mutates caller-owned page buffers in place; Mobius never
allocates or manages pages (onnx-genai-kv stays the sole cache authority).
The graph binds caller-owned block_table / slot_mapping / cumulative +
past lengths / per-layer LATENT key_cache, aliases the cache in place, and
derives token positions from the length tensors (no position_ids input).
kv_b_proj is absorbed into the query/output projections at weight-apply
time (numeric contract mirrors the onnx-genai equivalence oracle).

Wiring:
- config flag `export_paged_attention` (default False)
- `components/_paged_mla.py`: geometry, typed eligibility, weight
  absorption, `PagedLatentMLA` component, `PagedCacheState`
- `CausalLMTask(paged_cache=True)`: caller-owned LATENT cache IO
- DeepSeek/GLM model integration + weight absorption
- CLI `--features paged-attention` (+ task resolution, mutual exclusion
  with static-cache / --task, typed eligibility error)

Tests (same commit):
- component: eligibility rejects, absorption, numpy decomposed-vs-absorbed
  LATENT parity (rel<1e-6), structural emission
- model-level: feature-off byte-identical, feature-on structural + exact
  op attrs/inputs/outputs/model IO, typed rejects (DSA/CSA/HCA/MTP/window),
  torch-tensor weight absorption
- CLI: feature plumbing + mutual-exclusion errors

No full-size performance claim; native runtime full-size verification is
separate. Requires ORT >=1.29 CUDA to execute (contrib op unknown to the
base onnx checker, so model-level assertions are structural).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown

Performance Comparison

Comparing 9d1c0bb8a17cfb

Model Metric Baseline Current Delta
bert (feature-extraction) model_size_bytes 359 KB 359 KB +0.0%
bert (feature-extraction) num_nodes 68 68 +0.0%
falcon model_size_bytes 364 KB 364 KB +0.0%
falcon num_nodes 66 66 +0.0%
gemma2 model_size_bytes 428 KB 428 KB +0.0%
gemma2 num_nodes 105 105 +0.0%
gpt2 model_size_bytes 388 KB 388 KB +0.0%
gpt2 num_nodes 54 54 +0.0%
llama model_size_bytes 425 KB 425 KB +0.0%
llama num_nodes 60 60 +0.0%
llama (static-cache) model_size_bytes 425 KB 425 KB +0.0%
llama (static-cache) num_nodes 56 56 +0.0%
mamba (ssm-text-generation) model_size_bytes 296 KB 296 KB +0.0%
mamba (ssm-text-generation) num_nodes 94 94 +0.0%
phi3 model_size_bytes 421 KB 421 KB +0.0%
phi3 num_nodes 58 58 +0.0%
phi3 (static-cache) model_size_bytes 421 KB 421 KB +0.0%
phi3 (static-cache) num_nodes 54 54 +0.0%
qwen2 model_size_bytes 425 KB 425 KB +0.0%
qwen2 num_nodes 60 60 +0.0%
qwen2 (static-cache) model_size_bytes 425 KB 425 KB +0.0%
qwen2 (static-cache) num_nodes 56 56 +0.0%
qwen3_5_moe (hybrid-text-generation) model_size_bytes 506 KB 506 KB +0.0%
qwen3_5_moe (hybrid-text-generation) num_nodes 264 264 +0.0%
qwen3_5_text (hybrid-text-generation) model_size_bytes 458 KB 458 KB +0.0%
qwen3_5_text (hybrid-text-generation) num_nodes 126 126 +0.0%
qwen3_5_vl (hybrid-qwen-vl) model_size_bytes 977 KB 977 KB +0.0%
qwen3_5_vl (hybrid-qwen-vl) num_nodes 428 428 +0.0%
t5 (seq2seq) model_size_bytes 836 KB 836 KB +0.0%
t5 (seq2seq) num_nodes 176 176 +0.0%
whisper (speech-to-text) model_size_bytes 1008 KB 1008 KB +0.0%
whisper (speech-to-text) num_nodes 128 128 +0.0%

No performance regressions.

@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown

🏗️ Architecture Diff

Comparing 9d1c0bb8a17cfb

Model Sub-model Changes Status
bert (feature-extraction) model 0
falcon model 0
gemma2 model 0
gemma4 (gemma4) decoder 0
gemma4 (gemma4) embedding 0
gemma4 (gemma4) vision_encoder 0
gemma4_text model 0
gpt2 model 0
llama model 0
llama (static-cache) model 0
mamba (ssm-text-generation) model 0
phi3 model 0
phi3 (static-cache) model 0
qwen model 0
qwen (static-cache) model 0
qwen2 model 0
qwen2 (static-cache) model 0
qwen2_moe model 0
qwen2_moe (static-cache) model 0
qwen3 model 0
qwen3 (static-cache) model 0
qwen3_5_moe (hybrid-text-generation) model 0
qwen3_5_text (hybrid-text-generation) model 0
qwen3_5_vl (hybrid-qwen-vl) decoder 0
qwen3_5_vl (hybrid-qwen-vl) embedding 0
qwen3_5_vl (hybrid-qwen-vl) vision_encoder 0
qwen3_moe model 0
qwen3_moe (static-cache) model 0
qwen3_next (hybrid-text-generation) model 0
t5 (seq2seq) decoder 0
t5 (seq2seq) encoder 0
whisper (speech-to-text) decoder 0
whisper (speech-to-text) encoder 0

No architecture changes detected.


Legend: ⚪ No change · 🔵 Minor (attrs/inits) · 🟡 Moderate (nodes added/removed) · 🔴 Major (interface changed)

…ad_size %16

Address the code-review BLOCKER: use_dsa defaults to True on every
ArchitectureConfig (and via the HF resolver), and is only set False by
--glm-full-attention (gated to glm_moe_dsa). Plain DeepSeek-V2/V3 never
reads use_dsa (its text model is always dense), so gating the DSA typed
rejection on use_dsa alone wrongly rejected real DeepSeek-V3 paged export.

DSA is now treated as active only when use_dsa is set AND an indexer is
actually configured (index_n_heads / index_head_dim / index_topk /
indexer_types). This keeps:
  - DeepSeek-V3 (use_dsa=True default, no indexer) -> eligible
  - GLM --glm-full-attention (use_dsa=False, vestigial indexer) -> eligible
  - GLM DSA active (use_dsa=True + indexer) -> typed reject

Also tighten the LATENT geometry gate to mirror the native validator's
check_rotary_caches (validate.rs:510): dense MLA always emits do_rotary=1
with cos/sin caches, so head_size must be a multiple of 16, which (given
rotary_dim %16) forces kv_lora_rank %16 == 0. Closes the gap where an
8-aligned-but-not-16-aligned latent width would pass export yet be
rejected at native load.

Tests: DSA-reject tests now configure an indexer; new regression that
DeepSeek-V3 with the default use_dsa=True still emits paged nodes; geometry
reject test now uses kv_lora_rank=8 (8- but not 16-aligned). All paged
component + model + CLI + GLM/DeepSeek suites pass; ruff 0.16.2 clean.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@justinchuby

Copy link
Copy Markdown
Member Author

Independent review round 1 — verdict: 1 BLOCKER (fixed), 1 should-fix (fixed), 1 nit

An independent reviewer (not Leon/Sapper) ran the full suite (84 paged/GLM/DeepSeek/CLI tests pass), ruff clean, and cross-checked the emitted com.microsoft::PagedAttention v1 schema against the merged native op (crates/onnx-runtime-ep-cuda/.../paged_attention.rs, crates/onnx-genai-paged-attention/src/validate.rs). Findings addressed in ee9dbdd:

BLOCKER (fixed): use_dsa defaults to True on every ArchitectureConfig and is only set False by --glm-full-attention (gated to glm_moe_dsa). Plain DeepSeek-V2/V3 never reads use_dsa (its text model is always dense), so gating the DSA typed rejection on use_dsa alone wrongly rejected real DeepSeek-V3 paged export. DSA is now treated as active only when use_dsa is set and an indexer is actually configured (index_n_heads/index_head_dim/index_topk/indexer_types). Result: DeepSeek-V3 (default use_dsa=True, no indexer) → eligible; GLM full-attention (use_dsa=False, vestigial indexer) → eligible; GLM DSA active → typed reject. Added a model-level regression proving DeepSeek-V3 with the default use_dsa=True still emits paged nodes.

SHOULD-FIX (fixed): Tightened the LATENT geometry gate to mirror the native validator's check_rotary_caches (validate.rs:510): dense MLA always emits do_rotary=1 with cos/sin caches, so head_size % 16 == 0, which given rotary_dim % 16 forces kv_lora_rank % 16 == 0. Closes the gap where an 8-aligned-but-not-16-aligned latent width would pass export yet be rejected at native load. Real GLM/DSV3 (kv_lora_rank=512) unaffected.

NIT (acknowledged): byte-identical feature-off test is close to tautological by construction (feature default-off); kept as a guard against accidental default flips.

No behavioral change when the feature is off (still byte-identical). No full-size perf claims. Native runtime full-size verification remains a separate gate. Do not merge — awaiting Gaff/Roy final approval.

…ess, real qk-norm gate

No-BC cleanup of the eligibility validator (no compat shims). Every field the
rejection probes is a declared ArchitectureConfig field, so replace the
defensive getattr(...) with direct attribute access (use_dsa, indexer_types,
compress_ratios, o_lora_rank, o_groups, hc_mult, num_nextn_predict_layers,
sliding_window) and the export_paged_attention reads in deepseek.py/glm_moe_dsa.py.
Behavior is identical: each getattr default matched the declared default.

Correctness fix (mandatory typed refusal): per-head QK-norm is consumed into
the canonical attn_qk_norm / attn_qk_norm_full fields at config-extraction time
(the HF use_qk_norm / qk_layernorm passthrough is not an ArchitectureConfig
field, so the old check was always False and unreachable). Reject on the real
fields instead — a qk-norm MLA would need the operator's q_norm/k_norm inputs,
which this slice does not claim. DeepSeek-V2/V3 and GLM full-attention leave
both False, so the targets are unaffected. The refusal is now reachable and
tested at both component and model level.

Remove the provably-dead head_sink / attention_sink branch: neither is an
ArchitectureConfig field, so the guard was always False. Coverage is subsumed —
the only sink-bearing MLA family (DeepSeek-V4) is already rejected by its
CSA/HCA fields, a non-MLA sink model (gpt-oss) is rejected by the MLA-geometry
check, and PagedLatentMLA structurally never emits a head_sink input. Documented
in place; no silent-miscompute path is introduced.

Tests: add component test_qk_norm_rejected (attn_qk_norm / attn_qk_norm_full)
and test_window_rejected; add a model-level qk-norm typed-reject case. Full
paged/GLM/DeepSeek/V4/tasks/CLI suites pass; ruff 0.16.2 clean.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@justinchuby
justinchuby force-pushed the squad/mobius-3b-paged-attention-export branch from 3a3eb2d to 8a17cfb Compare August 24, 2026 22:41
@justinchuby

Copy link
Copy Markdown
Member Author

APPROVE 3B FOR MERGE

Independently reviewed at exact HEAD 75b5bf99223b31f0bc18e6b6f70df16507a76c6f (git rev-parse confirmed). Read-only; no edits/merge performed.

Op-contract verification (against onnx-genai crates/onnx-genai-paged-attention, current origin/main)

  • Input order (17): cross-checked PagedLatentMLA.forward's positional op.PagedAttention(...) call 0–16 against PagedAttentionInputs field order in params.rs (excluding the two derived *_storage_dtype fields) — exact match: query, key, value, key_cache, value_cache, cumulative_sequence_length, past_seqlens, block_table, cos_cache, sin_cache, slot_mapping, head_sink, q_norm_weight, k_norm_weight, k_scale, v_scale, attention_metadata.
  • Outputs (3): output, key_cache_out, _value_cache_out matches provider.rs's output-1-aliases-input-3 in-place LATENT semantics.
  • Attrs: kv_cache_layout="LATENT", kv_num_heads=1, explicit scale, v_head_size=kv_lora_rank, rotary_offset=kv_lora_rank, do_rotary=1, rotary_interleaved all line up with PagedAttentionAttributes and validate.rs::check_inputs's LATENT-specific branch (value_cache must be absent, kv_num_heads==1, head_sink/q_norm/k_norm absent, v_scale/v_quant/v_cache_dtype unset) — every one of these is honored by the emitted node (all corresponding inputs passed None).
  • Geometry gate: PR's head_size % 8, kv_lora_rank % 8, qk_rope_head_dim % 16(or 0), rotary_offset % 8 constraints match validate.rs exactly (rotary_dim is cos_cache-derived, never trusted as an attribute — correctly never emitted). DEFAULT_PAGED_BLOCK_SIZE=16 matches onnx-genai-kv::MIN_PAGED_BLOCK_SIZE. key_cache shape [num_blocks, block_size, 1, head_size] matches check_kv_cache.

Independently reproduced (not just read)

  • Ran the full _paged_mla_test.py + paged_mla_export_test.py (38 passed) and full tests/cli_test.py (46 passed, incl. the 3 new paged-attention CLI tests).
  • Ran the entire mobius/models/ suite (558 passed, 4 skipped, 2 pre-existing unrelated failures — qwen_image_test.py golden-file FileNotFoundErrors, reproduced identically against main@3474c96 when invoked the same way from src/; confirmed cwd-relative-path artifact, unrelated to this PR).
  • ruff check / ruff format --check scoped to all 11 touched files: clean.
  • Clean merge against origin/main (0665542, Fuse GLM native-block routed experts into sparse pkg.nxrt::BlockQuantizedMoE (+ build_from_gguf wiring) #591 BQMoE fusion) — zero conflicts, zero file overlap.
  • Directly executed extra scenarios beyond the PR's own tests to stress the "typed reject, never silent fallback" claim:
    • GLM use_dsa=True + flag on → rejects (ValueError, DSA/IndexShare message).
    • DeepSeek compress_ratios=[4,8] → rejects (CSA/HCA message).
    • DeepSeek quantized cache dtype (INT8) → rejects.
    • GLM use_dsa=False (full-attention) + flag on → constructs and exports fine, 2 PagedAttention nodes, composes cleanly with the model's MoE FFN layers (no BQMoE/Fuse GLM native-block routed experts into sparse pkg.nxrt::BlockQuantizedMoE (+ build_from_gguf wiring) #591 overlap — verified by building a real GLM-MoE-DSA-off graph end-to-end, 234 nodes, no crash, no shared op-name collision).
    • _absorb_paged_mla_weights / PagedLatentMLA's untested q_proj (no-lora, e.g. DeepSeek-V2-Lite-style) branch: manually exercised — shapes and construction are correct. Not a bug, but genuinely absent from the PR's own test suite (see findings).
  • Numerical parity tests (TestNumericalParity) are real: decomposed-vs-absorbed LATENT comparison over full prefill+decode causal-attention progression, rel-err < 1e-6, 3 seeds × 2 RoPE-interleave settings + real GLM/DeepSeek head dims (kv_lora_rank=512, qk_rope=64, qk_nope=128). This is a legitimate oracle-equivalence proof, not a shape-only check.

Findings (non-blocking; recommend as explicit follow-ups)

  1. Dead-code rejection branches: paged_attention_rejection's head_sink/attention_sink/use_qk_norm/qk_layernorm getattr(..., False) checks reference config attributes that do not exist anywhere in ArchitectureConfig or any model in this repo (grep-verified) and have zero test coverage. This is currently harmless: DeepSeek-V4 (which has an unconditional, always-present attn_sink parameter in DeepSeekV4Attention.__init__) is already reliably excluded by three independent, real, tested signals — assert config.o_lora_rank is not None (a hard, unconditional assertion for V4 construction to succeed at all), config.sliding_window (V4-Flash always sets this per the code's own comment), and compress_ratios (non-empty list, even all-zero, is still truthy). But the PR body/docstring's claim that "head_sink, qk-norm... raise a typed reason" is currently aspirational, not verified. Recommend either wiring these to a real field once one exists, or removing/annotating the dead branches so the claim isn't overstated.
  2. q_proj (no-lora Q) path is untested in both _paged_mla_test.py and paged_mla_export_test.py — only the q_lora_rank-set (DeepSeek-V3/GLM-5.2) branch is exercised. Manually verified correct, but should get a real test if DeepSeek-V2-Lite-style (no q-lora) checkpoints are in scope.
  3. --features paged-attention --runtime ort-genai is unguarded in _validate_ort_genai_compatibility (would silently emit a genai_config.json using past_key_names/past_value_names templates that don't match the paged cache I/O names). Low practical risk — this targets Microsoft's separate onnxruntime-genai consumer, not the onnx-genai native Rust runtime this slice is built for, and the PR makes no claim of --runtime ort-genai support.
  4. Cosmetic: a duplicated, unreachable l % 8 != 0 check in paged_attention_rejection (harmless dead branch); the "batch + 1" dim-param string for cumulative_sequence_length is an unrelated symbolic-dim label to batch (fine for ONNX, just not textually correlated).

None of the above affect correctness for the PR's stated target configurations (DeepSeek-V2/V3 dense MLA with q-lora, GLM-5.2 --glm-full-attention), and the default-off flag means zero blast radius for anyone not opting in.

Verdict

APPROVE 3B FOR MERGE. Property-based eligibility (verified non-model-name-gated via the DeepSeek-V3-passes-without-name-check test + my own repro), exact op contract, byte-identical feature-off, real typed rejection for every currently-constructible incompatible model (GLM DSA, DeepSeek CSA/HCA/o-groups/hc_mult/MTP/quantized-cache/sliding-window), single cache authority (Mobius allocates no pages; PagedCacheState is 100% caller-owned), and no #591/BQMoE overlap are all independently confirmed, not just trusted.

Default-off seam may merge before full-size A/B. Exact next full-size-GLM/DeepSeek gate before any full-size checkpoint export+correctness claim is admissible:

  1. A real end-to-end functional test that loads a full-size (or shape-faithful reduced) DeepSeek-V3/GLM-5.2 checkpoint, exports with --features paged-attention, and executes the graph under ORT ≥ 1.29 CUDA (not just structural/attr assertions — actual kernel execution) with output parity vs. the dense-MLA baseline.
  2. A native onnx-genai-runtime-side integration test that consumes an actual Mobius-exported paged-attention graph end-to-end (today the Rust-side oracle/#1978 kernel tests and this PR's Mobius-side tests are verified independently of each other, not cross-validated against a shared real artifact).
  3. Close finding Test infra L7 #2 above (q_proj/no-lora coverage) if DeepSeek-V2-Lite-style checkpoints are in scope for full-size validation.
  4. No full-size perf claim until measured per cuda-perf-measurement/measurement-discipline skill discipline — this PR correctly makes none.

@justinchuby

Copy link
Copy Markdown
Member Author

Follow-up: no-backward-compat honesty cleanup (8a17cfb)

Per the no-BC-during-development directive, tightened the eligibility validator with clean breaking access (no compat shims), keeping correctness / property gates / default-off / typed refusals mandatory. All in-repo callers/fixtures/tests updated atomically.

Direct declared-field access. Every field paged_attention_rejection probes is a declared ArchitectureConfig field, so the defensive getattr(config, ..., default) calls are replaced with direct attribute access (use_dsa, indexer_types, compress_ratios, o_lora_rank, o_groups, hc_mult, num_nextn_predict_layers, sliding_window), plus the export_paged_attention reads in deepseek.py/glm_moe_dsa.py. Behavior is identical — each getattr literal default matched the declared default. (_has(name) stays: it is a by-name "present and positive" helper for the int | None geometry fields, not a shim.)

Correctness fix — real qk-norm refusal. Per-head QK-norm is consumed into the canonical attn_qk_norm / attn_qk_norm_full fields at config extraction (the HF use_qk_norm / qk_layernorm passthrough is not an ArchitectureConfig field). The prior check probed those always-absent names, so it was unreachable — a qk-norm MLA would have slipped through and silently missed the operator's q_norm/k_norm inputs. Now rejects on the real fields; DeepSeek-V2/V3 and GLM full-attention leave both False, so targets are unaffected. Reachable + tested at component and model level.

Removed a provably-dead branch. head_sink / attention_sink are never ArchitectureConfig fields, so that guard was always False. Coverage is subsumed and documented: the only sink-bearing MLA family (DeepSeek-V4) is already rejected by its CSA/HCA fields, a non-MLA sink model (gpt-oss) is rejected by the MLA-geometry check, and PagedLatentMLA structurally never emits a head_sink input. No silent-miscompute path is introduced.

Tests: added test_qk_norm_rejected (attn_qk_norm / attn_qk_norm_full) and test_window_rejected component tests, plus a model-level qk-norm typed-reject case; the DeepSeek-V3 default-use_dsa=True regression now asserts directly. Full paged/GLM/DeepSeek/V4/tasks/CLI suites pass (247 across the sweeps); ruff 0.16.2 clean. Default-off remains byte-identical.

Still a DRAFT — do not merge; awaiting Gaff or Roy final approval.

@justinchuby

Copy link
Copy Markdown
Member Author

Roy — Final re-review of PR #599 at HEAD 8a17cfb4d668104ea92b52c3a9e6d98957905fde

Scope: delta 75b5bf9..8a17cfb (the independent-review fixup: ee9dbdd DSA/geometry fix + 8a17cfb honesty cleanup) plus integration against latest origin/main. My prior 75b5bf9 approval stands corrected — the independent reviewer caught a real functional bug my own repro had masked. Verified all 5 requested points independently, not by trusting the PR's own tests.

1. Direct ArchitectureConfig field access — VALID everywhere

ArchitectureConfig (_configs/_base.py:396) is a single, non-subclassed @dataclass used uniformly by every model. Every field now accessed directly (sliding_window, attn_qk_norm, attn_qk_norm_full, o_groups, o_lora_rank, compress_ratios, hc_mult, num_nextn_predict_layers, use_dsa, export_paged_attention, indexer_types) is a genuinely declared field with a default. Only remaining getattr is inside _has(), a justified DRY helper for the 7 int|None MLA-geometry/indexer fields (all real declared fields) — not a defensive shim. No AttributeError risk for any model_type.

2. qk-norm refusal — reachable, correct, no false-reject

Old check probed getattr(config, "use_qk_norm", False) — a literal field that never exists post-extraction on ArchitectureConfig, i.e. permanently dead/unreachable code. Fix reads the real canonical attn_qk_norm / attn_qk_norm_full fields, which are derived at config-extraction time (_base.py:~832-852) from a model_type allowlist (gemma3/qwen3/olmo families) or raw HF use_qk_norm passthrough. Neither DeepSeek nor GLM model_types appear in either allowlist and neither sets a raw use_qk_norm — both fields correctly default False for PR targets. No false-reject for DeepSeek-V2/V3/GLM-5.2; check is now genuinely reachable for any checkpoint that does set qk-norm.

3. Dead head_sink branch removal — coverage preserved

Removal is safe: DeepSeek-V4 (the only model_type where head_sink/attention-sink semantics could matter) is already independently excluded from paged-MLA eligibility via its CSA/HCA fields (established in the earlier Cycle-1 review), and PagedLatentMLA structurally never emits a head_sink input regardless. No coverage gap opened by the removal.

4. head_size % 16 fix — independently confirmed against native source

Verified directly against crates/onnx-genai-paged-attention/src/validate.rs::check_rotary_caches (onnx-genai repo, line 510): head_size % 16 != 0 is the real, exact native-loader constraint — not % 8. The old % 8 check would have let an 8-aligned-but-not-16-aligned kv_lora_rank export successfully from Mobius, then fail at native ORT load — a real silent-broken-artifact risk, now closed correctly.

5. New/updated tests — non-vacuous

  • test_deepseek_v3_default_use_dsa_is_eligible: proves use_dsa=True (true default) + no indexer configured → eligible. This is the exact regression guard for the BLOCKER.
  • test_dsa_indexshare_rejected: updated to also set indexer fields (since use_dsa alone no longer triggers rejection) — still correctly rejects when DSA is actually active.
  • test_qk_norm_rejected: independently tests attn_qk_norm=True and attn_qk_norm_full=True, plus a baseline-eligible case.
  • test_window_rejected: tests sliding_window=64 rejects and sliding_window=None doesn't.
  • test_geometry_constraints_rejected: updated to the new 16-boundary (kv_lora_rank=8, qk_rope_head_dim=16 → head_size=24, correctly rejected as the exact 8-aligned-not-16-aligned edge case).
  • paged_mla_export_test.py::_deepseek_config — critically, the explicit use_dsa=False test override was removed; test_deepseek_v3_emits_paged_not_name_gated now asserts config.use_dsa is True before checking paged nodes emit, directly exercising the true production default rather than an artificially-forced config.

Each new/changed test has both a rejecting case and a non-rejecting baseline/None case — not vacuous.

6. Feature-off / no regression

Feature-off code paths untouched by this delta. Full regression run:

  • mobius/components/_paged_mla_test.py + mobius/models/paged_mla_export_test.py: 41/41 passed
  • mobius/models/deepseek_test.py, deepseek_v4_test.py, deepseek_v4_flash_test.py, glm_moe_dsa_test.py, mobius/tasks/: 163/163 passed
  • tests/cli_test.py: 46/46 passed
  • Full mobius/models/ suite: 558 passed, 2 failed, 4 skipped. The 2 failures (qwen_image_test.py::test_edit_vae_matches_diffusers_on_real_source_image, ::test_deterministic_l4_l5_image_edit_golden) are pre-existing FileNotFoundErrors for missing local testdata assets in an unrelated diffusion image-editing model, not touched by this PR in any way — confirmed unrelated, and the 558-pass count matches the prior baseline exactly.
  • ruff check + ruff format --check on all 5 touched files: clean.
  • Clean merge against latest origin/main (includes #578, #581, #576) — zero conflicts, re-verified.

Verdict: APPROVE FINAL HEAD

All four requested verification points are independently confirmed correct, the fix genuinely closes the missed BLOCKER (production DeepSeek-V3 configs with the true use_dsa=True default would previously have been wrongly rejected), the should-fix (16-alignment) is exactly correct per native source, the honesty cleanup introduces no new risk, and no regressions were found across 808 total tests run plus lint/format. PR #599 is approved for merge at 8a17cfb4d668104ea92b52c3a9e6d98957905fde.

— Roy

@justinchuby
justinchuby marked this pull request as ready for review August 25, 2026 00:05
@justinchuby
justinchuby requested review from a team and a lite review from Copilot August 25, 2026 00:05
@justinchuby
justinchuby merged commit 59a595e into main Aug 25, 2026
23 of 24 checks passed
@justinchuby
justinchuby deleted the squad/mobius-3b-paged-attention-export branch August 25, 2026 00:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds opt-in LATENT PagedAttention export for dense MLA models, with paged-cache wiring, geometry validation, weight absorption, and CLI support.

Changes:

  • Adds LATENT com.microsoft::PagedAttention graph emission.
  • Integrates paged cache handling across tasks, builders, and supported models.
  • Adds structural, parity, rejection, and CLI tests.

Final review findings:

  • src/mobius/__main__.py: Reject or support ort-genai paged-cache ABI (critical, 1 vote).
  • src/mobius/components/_paged_mla.py: Reconcile %16 gates with the advertised runtime contract (moderate, 2 votes).
  • src/mobius/integrations/transformers/_builder.py: Select the paged task when enabled (critical, 4 votes).
  • src/mobius/models/deepseek.py: Handle or reject quantized weight absorption (critical, 4 votes).
  • src/mobius/tasks/_causal_lm.py: Use packed token/logit shapes for varlen paged attention (critical, 1 vote).
  • src/mobius/tasks/_causal_lm.py: Preserve or reject paged mode with prefix pruning (critical, 2 votes).

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.

Show a summary per file
File Summary
tests/cli_test.py Tests paged-attention CLI plumbing and conflicts.
src/mobius/tasks/_causal_lm.py Adds paged cache graph inputs and outputs.
src/mobius/models/paged_mla_export_test.py Tests structural export and rejection cases.
src/mobius/models/glm_moe_dsa.py Handles GLM dense-attention eligibility.
src/mobius/models/deepseek.py Integrates paged MLA and weight absorption.
src/mobius/integrations/transformers/_builder.py Adds builder support and validation.
src/mobius/components/_paged_mla.py Implements LATENT attention, validation, and absorption.
src/mobius/components/_paged_mla_test.py Tests geometry and numerical parity.
src/mobius/components/__init__.py Exposes paged MLA components.
src/mobius/_configs/_base.py Adds paged-attention configuration.
src/mobius/__main__.py Adds CLI feature handling and validation.
Suppressed comments (10)

src/mobius/main.py:289

  • The CLI allows --features paged-attention,prune-prefill-prefix, but build_from_module() then reconstructs this task through _enable_prefill_prefix_pruning_task() without preserving paged_cache=True. The paged model consequently receives ordinary tuple caches and fails in _forward_paged() when dataclasses.replace() is called on them. Reject this feature combination or preserve the paged task and implement packed per-request last-token selection.
        task = CausalLMTask(paged_cache=True)

src/mobius/main.py:277

  • export_paged_attention is resolved here, but the diffusers and NeMo source-dispatch branches below call their own builders and return without passing or validating this flag. Thus --features paged-attention silently succeeds while exporting a non-paged diffusion/NeMo package, instead of producing the required typed incompatibility error. Add a fail-fast check for these source types before dispatch.
    # PagedAttention (LATENT dense-MLA) export uses the paged-cache task with
    # caller-owned page buffers. It is a distinct cache authority, so it cannot
    # be combined with the static-cache task or an explicit --task.
    export_paged_attention = getattr(args, "export_paged_attention", False)

src/mobius/main.py:283

  • Paged and FP8-cache features are not mutually excluded here. With CUDA, the existing FP8 pass sees no GroupQueryAttention in this graph and raises after the paged graph is built; on unsupported EPs it only warns and leaves the cache unquantized. Either way --features paged-attention,fp8-kv-cache is not a valid typed-rejected request as required for quantized cache modes. Reject the combination before build.
    if export_paged_attention:
        if static_cache_params is not None:
            raise SystemExit(
                "Error: --features paged-attention cannot be combined with "
                "--features static-cache."
            )

src/mobius/main.py:437

  • With --runtime onnx-genai, this newly exposed package reaches the generic decoder workflow builder, which does not classify block_table, slot_mapping, cumulative_sequence_length, or past_seqlens as paged state. It treats block_table as a fallback position input and raises on the other unsupported inputs, so this export cannot write inference_metadata.yaml. Add a paged control/state mapping or reject this runtime explicitly.
            export_paged_attention=export_paged_attention,

src/mobius/components/init.py:212

  • These new symbols are re-exported from mobius.components below but are missing from the module's __all__, unlike the existing public cache/component symbols. That makes wildcard imports and API introspection omit PagedLatentMLA/PagedCacheState and the new helpers; add the intended public names to __all__, or keep them private consistently.
from mobius.components._paged_mla import (
    PagedCacheState as PagedCacheState,
)

src/mobius/integrations/transformers/_builder.py:259

  • fp8_kv_cache is accepted alongside export_paged_attention, but the FP8 pass only converts GroupQueryAttention; this graph contains PagedAttention, so optimization warns/ignores the request and leaves a non-FP8 cache instead of typed-rejecting the unsupported combination. Reject the combination before building (for both CLI and direct builder APIs).
    if export_paged_attention:
        from mobius.components._paged_mla import paged_attention_rejection

        config = dataclasses.replace(config, export_paged_attention=True)
        reason = paged_attention_rejection(config)
        if reason is not None:
            raise ValueError(
                "export_paged_attention=True (--features paged-attention) is not "
                f"supported for model_type '{model_type}': {reason}"
            )

src/mobius/integrations/transformers/_builder.py:259

  • The eligibility predicate admits any config with matching MLA geometry, but not every such model implements the paged forward/cache contract (for example, LongCat Flash has MLA-shaped config fields but its attention layers still consume normal two-part KV state). After task selection is corrected, this produces a paged task feeding PagedCacheState into an unsupported implementation. Add an implementation capability check/protocol so unsupported architectures receive a typed rejection instead of a runtime type/shape failure.
    if export_paged_attention:
        from mobius.components._paged_mla import paged_attention_rejection

        config = dataclasses.replace(config, export_paged_attention=True)
        reason = paged_attention_rejection(config)
        if reason is not None:
            raise ValueError(
                "export_paged_attention=True (--features paged-attention) is not "
                f"supported for model_type '{model_type}': {reason}"
            )

src/mobius/tasks/_causal_lm.py:562

  • These new paged control inputs are not handled by the existing ONNX GenAI metadata builder. It only admits a rank-2 token/attention/position input; block_table becomes the fallback position input, while rank-1 slot_mapping, cumulative_sequence_length, and past_seqlens remain unsupported and write_onnx_genai_config() raises. Add a dedicated paged control/state-service contract (including paged cache layout), or reject --runtime onnx-genai for this export instead of emitting an unusable package.
    block_table = builder.input(
        "block_table", dtype=ir.DataType.INT32, shape=[batch, max_blocks]
    )
    slot_mapping = builder.input("slot_mapping", dtype=ir.DataType.INT32, shape=[num_tokens])
    cumulative_sequence_length = builder.input(
        "cumulative_sequence_length", dtype=ir.DataType.INT32, shape=["batch + 1"]
    )
    past_seqlens = builder.input("past_seqlens", dtype=ir.DataType.INT32, shape=[batch])

src/mobius/tasks/_causal_lm.py:297

  • Unlike the normal path above, _build_paged invokes the module outside prefill_prefix_pruning(...) and never calls _validate_pruned_logits(...). A CausalLMTask(paged_cache=True, prune_prefill_prefix=True) therefore silently emits full-sequence logits, breaking the documented prefix-pruning option even if task reconstruction is fixed. Apply the same context and validation around the paged module call.
        result = module(
            op,
            input_ids=input_ids,
            attention_mask=None,
            position_ids=None,
            past_key_values=paged_states,
        )

src/mobius/tasks/_causal_lm.py:301

  • The ordinary CausalLMTask path preserves a third model result and registers hidden_states.* for output_layer_indices, but this path binds that result to _intermediate and drops it. Selecting paged attention therefore changes the model's declared outputs for any compatible module/config that requests intermediate hidden states. Mirror the normal unpacking and _register_intermediate_hidden_states handling.
        if len(result) == 3:
            logits, present_key_values, _intermediate = result
        else:
            logits, present_key_values = result

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/mobius/__main__.py
"prune-prefill-prefix": "prune_prefill_prefix",
"text-only": "text_only",
"glm-full-attention": "glm_full_attention",
"paged-attention": "export_paged_attention",
Comment on lines +256 to +259
if head_size % 16 != 0:
return f"PagedAttention LATENT requires head_size % 16 == 0; got {head_size}."
if l % 16 != 0:
return f"PagedAttention LATENT requires latent_dim (kv_lora_rank) % 16 == 0; got {l}."
Comment on lines +250 to +259
if export_paged_attention:
from mobius.components._paged_mla import paged_attention_rejection

config = dataclasses.replace(config, export_paged_attention=True)
reason = paged_attention_rejection(config)
if reason is not None:
raise ValueError(
"export_paged_attention=True (--features paged-attention) is not "
f"supported for model_type '{model_type}': {reason}"
)
Comment on lines +564 to +565
if self.config.export_paged_attention:
renamed = self._absorb_paged_mla_weights(renamed)
Comment on lines +283 to +289
paged_states = _make_paged_cache_inputs(
builder,
config.num_hidden_layers,
geom.head_size,
config.dtype,
batch,
)
Comment on lines 97 to +105
static_cache: bool = False,
paged_cache: bool = False,
max_seq_len: int | None = None,
prune_prefill_prefix: bool = False,
):
if static_cache and paged_cache:
raise ValueError("static_cache and paged_cache are mutually exclusive.")
self._static_cache = static_cache
self._paged_cache = paged_cache
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