Skip to content

fix(loading): gemma4_unified quantized sanitize breaks dequantize/split equivalence #1074

Description

@inureyes

Problem / Background

loading::vlm::gemma_unified::tests::unified_sanitize_quantized_split_dequant_equivalence fails on main at commit e9f1f191.

The failing assertion is at src/loading/vlm_gemma_unified_tests.rs:354:

assertion `left == right` failed: dequantize(split(gate)) must equal split(dequantize)(gate)
  left: 3.7108002
 right: 0.0

left is max_abs_diff(&gate, &ref_gate), so the gate_proj leg produced by sanitize_gemma4_unified_weights dequantizes to something structurally different from the reference split(dequantize(gate_up_proj)). The up_proj assertion that follows is never reached, so its status is unknown.

The rest of the workspace is green: make verify-test reports 7439 passed and this 1 failed, and both make verify-fmt and make verify-clippy (--workspace --all-targets --features metal,accelerate -- -D warnings) pass.

Already ruled out (do not re-run these)

  1. Not the Rust 1.97.1 toolchain bump (chore(ci): bump the pinned Rust toolchain to 1.97.1 #1066). Holding the source constant at e9f1f191 and compiling with the previously pinned toolchain reproduces the failure with byte-identical values (3.7108002 against 0.0):

    cargo +1.93.1-aarch64-apple-darwin test --profile test-fast --features metal,accelerate --lib unified_sanitize_quantized_split_dequant_equivalence
    

    The compiler version is not the variable.

  2. Not an f32 reassociation tolerance problem. Reassociation differences of this kind land around 1e-6. A left-hand value of 3.71 against an expected 0.0 is a structural disagreement, not a rounding one. fix(tests): tolerate f32 reassociation in the mllama tile-selection parity test #1067 did fix a genuine reassociation case in the mllama tile-selection parity test immediately after the toolchain bump, which makes this failure easy to misfile as the same class. It is not.

  3. Not epic epic: add Florence-2 (florence2) VLM support #850 (Florence-2). Its five merged PRs (feat(models): Florence-2 BART seq2seq engine and text core #1060, feat(models): Florence-2 DaViT vision backbone #1063, feat(models): Florence-2 vision-language fusion and full weight loading #1064, feat(models): Florence-2 processor, task prompts, and location tokens #1069, feat(models): Florence-2 end-to-end integration and real-checkpoint validation #1071) touch no gemma and no quantization code. src/loading/vlm_gemma_unified.rs and src/loading/vlm_gemma_unified_tests.rs were last modified by feat: add video input support for Gemma 4 Unified (gemma4_unified) #400 (d35ef06f), well before that epic.

Why this may have gone unnoticed

The root cargo test gate historically covered only one of the workspace members (#1007), and the nightly run has not completed within its timeout (#1000). This test may have been red for some time and is only now visible because the workspace-wide make verify-test gate exercises it.

Proposed Solution

  1. Bound the regression window first. Run the single test at the merge commits of feat: add video input support for Gemma 4 Unified (gemma4_unified) #400 (d35ef06f) and fix: split quantized fused MoE experts in gemma4_unified sanitize #156 (6c1861e1) to find when it last passed. This is the cheapest first step and it decides everything downstream.
  2. Determine the root cause: whether sanitize_gemma4_unified_weights splits quantized fused MoE experts in a way that no longer commutes with dequantization for the gate projection (a real sanitize defect), or whether the test expectation is stale relative to a deliberate later change (a stale expectation). The commuting property is what fix: split quantized fused MoE experts in gemma4_unified sanitize #156 originally implemented, so the split must partition weight, scales, and biases at the same output-axis half boundary with no quantization group straddling.
  3. Land the fix, or the corrected expectation with a justification for why the previous invariant no longer holds.
  4. Confirm the up_proj leg as well, since the current failure masks it.

Acceptance Criteria

  • Regression window identified: the last commit at which unified_sanitize_quantized_split_dequant_equivalence passed is named.
  • Root cause stated explicitly as either a real sanitize defect or a stale test expectation, with the evidence for that classification.
  • Fix (or corrected expectation) is merged, covering both the gate_proj and up_proj legs.
  • make verify-test is green on main.
  • make verify-fmt and make verify-clippy remain green.

Technical Considerations

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area:modelsModel architectures, weights, loading, metadatapriority:highHigh prioritystatus:doneCompletedtype:bugBug fixes, error corrections, or issue resolutions

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions