Skip to content

tests/zamba2: set n_mamba_heads in the fixture, so the test means the same thing on transformers 4 and 5 (engine #1036) - #232

Open
bong-water-water-bong wants to merge 1 commit into
1bit/hrx-vulkan-patchedfrom
1bit/zamba2-test-mamba-heads
Open

bong-water-water-bong wants to merge 1 commit into
1bit/hrx-vulkan-patchedfrom
1bit/zamba2-test-mamba-heads

Conversation

@bong-water-water-bong

Copy link
Copy Markdown

Refs 1bit-MONSTER/engine#1036. Follow-up to #229, fixing the finding in its review: the new test depended on the installed transformers version without saying so.

Cause

The fixture passed mamba_headdim=16 but not n_mamba_heads, which defaults to 8:

  • transformers 4.57 derives headdim from n_mamba_heads (64 / 8 = 8). It ran 8 heads of 8, not the shape the fixture wrote. HF and the converter still agreed, so the test passed.
  • transformers 5.17 keeps headdim 16 and 8 heads. HF builds an ssm_in 168 wide, but the converter infers 64 / 16 = 4 heads and expects 164, so every GGUF is refused before the code under test runs.

Measured directly:

Zamba2Config(hidden_size=32, mamba_expand=2, mamba_headdim=16)
  4.57.6: n_mamba_heads 8, mamba_headdim 8
  5.17.0: n_mamba_heads 8, mamba_headdim 16
... with n_mamba_heads=4 added, both versions: 4, 16

Change

tests/test-zamba2-convert.py: pass n_mamba_heads=2 * HIDDEN // 16 next to mamba_headdim=16 (+2 lines, one of them a comment). The fixture now has the same shape on both versions.

Proof (strixhalo, CPU only)

Before, from the #229 review (5.17.0, same box; fails the same way on the base and the fixed build):

FAIL blocks1/blocks1-attn/blocks2/blocks3: loads and evaluates on the CPU (exit 1)
  check_tensor_dims: tensor 'blk.0.ssm_in.weight' has wrong shape; expected 32, 164, got 32, 168, 1, 1

My own run of the old fixture under 5.17 was stopped by a low-memory reap before it started. It will be added here when memory allows.

After, this branch:

transformers 4.57.6, fixed loader (#229):  12/12 ok, PASS
transformers 5.17.0, fixed loader (#229):  12/12 ok, PASS
transformers 5.17.0, unfixed loader (#218 head, c2 build):
  FAIL blocks1-attn: loads and evaluates on the CPU (exit -6)    <- the #229 defect, still caught
  other 11 checks ok (incl. move control exit -6, swap controls 40 mismatches)
  FAILED: 1

🤖 Generated with Claude Code

… same thing on transformers 4 and 5 (engine ggml-org#1036)

The fixture passed mamba_headdim=16 without n_mamba_heads (default 8). transformers 4.57 derives headdim from n_mamba_heads
(64 / 8 = 8), so it ran 8 heads of 8, not the shape written. transformers 5.17 keeps headdim 16 and 8 heads: HF builds an
ssm_in 168 wide while the converter infers 64 / 16 = 4 heads and expects 164, and every --bin case fails with
"blk.0.ssm_in.weight has wrong shape; expected 32, 164, got 32, 168" on any build. Setting both makes the two agree.

Found in the review of #229.

  transformers 4.57.6, fixed loader:  PASS 12/12
  transformers 5.17.0, fixed loader:  PASS 12/12   (before: 4 FAIL, wrong shape)

Assisted-by: Claude Opus 5.5
@bong-water-water-bong

Copy link
Copy Markdown
Author

ready for review at 9b29684

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VERDICT: READY at 9b29684b5bdaea56c69bf51b39f1c9e6b6aeeb03. The fixture now pins its own shape on both transformers lines; reproduced on the two versions on this box.

Zamba2Config(hidden_size=32, mamba_expand=2, mamba_headdim=16[, n_mamba_heads=2*32//16]):

transformers old fixture (heads × headdim) this PR
5.17.0 (~/venvs/d3ref) 8 × 16, inconsistent with the converter's 64/16 = 4 heads, so the GGUF is refused (ssm_in 168 vs 164) 4 × 16
4.57.6 (~/venvs/quark013) 8 × 8: headdim silently derived from the default 8 heads, not the shape the fixture wrote 4 × 16

So the test was passing on 4.57 against a different model than the one it described, and failing on 5.17 before the code under test ran. With n_mamba_heads = 2·HIDDEN/16, both give the written shape. +2 lines in tests/test-zamba2-convert.py, one of them the comment.

Not re-run: the 12/12 convert-and-load runs on both versions and the "unfixed loader still caught" row. They need a fork build, and the config check above is the claim that decides them.

@bong-water-water-bong

Copy link
Copy Markdown
Author

No verdict from me — the READY at 9b29684b already covers this head and I do not want to add a second gate entry. One observation
from checking the fix independently, because it is the kind of number a later reader will have to re-derive:

HIDDEN = 32 in this fixture, so n_mamba_heads=2 * HIDDEN // 16 evaluates to 4, and the invariant behind it is

n_mamba_heads * mamba_headdim  ==  hidden_size * mamba_expand        4 * 16 == 32 * 2 == 64   (the ssm_in width)

That equality is why pinning the heads makes 4.x and 5.x agree — the two versions disagree about which of the pair their default
derives, not about the width — and it is worth one clause in the comment beside the expression, next to the version note that is
already there. 2 * HIDDEN // 16 reads as a magic number otherwise, and the constant it encodes (headdim 16, expand 2) is set five
lines above.

Rest checked and sound: the diagnosis matches the field semantics (4.57 derives headdim from heads; 5.17 keeps them unrelated, so
HF builds a 168-wide ssm_in where the converter infers 64/16 = 4 heads and expects 164), and the fix makes the fixture say what it
means rather than depending on the installed transformers. The test itself needs torch + transformers, neither of which is in this
box's python, so the version matrix in the body is the evidence and it is the right shape of evidence for a version-dependent
fixture.

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