Repository navigation
[serge] Fix 2 integration tests for model minimax failing with output_mismatch (tensor values differ (2)) - #48515
Conversation
…put_mismatch` (tensor values differ (2))
|
[For maintainers] Suggested jobs to run (before merge) run-slow: minimax |
CI recapDashboard: View test results in Grafana |
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
Root cause investigation (via SSH runner on A10G, CUDA 8.6, PyTorch 2.13.0+cu130)Root commit confirmed by
|
Investigation findingsAfter digging into the root cause, here is what happened: Root commit: PR #41580 (ArthurZucker, Why the values changed: Arthur's PR did two things simultaneously:
The forward pass is mathematically equivalent, but a batched fused matmul across all experts accumulates floating-point rounding in a different order than sequential per-expert matmuls on GPU → tiny bfloat16-level logit differences. Not broken logic, just a different computation path. Further drift after Arthur's commit: The values on current main differ slightly from Arthur's commit values too — this is normal and expected (PyTorch version upgrades, TF32 attribute changes, cuBLAS kernel selection, etc.). In any case, we have no single passing day after Arthur's commit ( Conclusion: Updating the expected logit values to match current main is the correct and sufficient fix. This PR is good to merge. |
|
Serge, this time you did a good job 🎉 but I'm still better! |
…put_mismatch` (tensor values differ (2)) (huggingface#48515) Co-authored-by: serge[bot] <serge[bot]@users.noreply.github.com>
serge's expectation updates mostly don't have the shape maintainers
accept. Of the serge PRs opened in Sept–Oct 2026, the 3 value updates
that touched only the CI runner's `("cuda", 8)` entry all merged cleanly
(huggingface/transformers#48515, #49068, #49076). The one that overwrote
`("cuda", None)` merged with a wrong value (#48580). The open queue
overwrote `(None, None)` (#47616, #49480), rewrote XPU values with no
XPU evidence (#48955), loosened tolerances (#49143) and edited plain
literals in place (#47840). The prompt already asked for a device key.
This PR enforces it.
**`reviewbot/expectation_keys.py`** parses each changed test file before
and after the patch (`ast`), maps every `Expectations({...})` block key
→ value, and reports:
- a change to, or removal of, any key other than the runner's `("cuda",
8)` / `("cuda", (8, 6))` (A10G, compute capability 8.6). Demotion is
allowed: an existing value moved unchanged to `(None, None)`, which is
what #48198 did;
- an added key other than the runner's, or a `(None, None)` value that
isn't an existing one;
- a runner value moving by more than 5% of the block's largest
magnitude. Accepted drift was 0.4% (#48515) and 3.6% (#49068, bf16 near
zero); the pvt groups that should have been refused moved 18% and 45%;
- a loosened `atol`/`rtol`, in any patch;
- in an expectation-only patch, an expected value or assertion changed
outside any `Expectations` block. Test-code fixes are left alone (#47697
renamed a config key).
**Wiring.** `_validate_patch` runs it after the patch applies, before
the brevity pass and the normalizer. A violation resets the worktree and
comes back as correction feedback (rule text included), using the same
`TASK_NORMALIZE_MAX_RETRIES` budget. If a violation survives the budget,
the PR still opens, with a "⚠️ Expectation change not in the maintainer
shape" section listing the problems. `TASK_EXPECTATION_KEY_CHECK=0`
turns the check off.
**Checked against real PRs.** Every serge PR since 2026-07-20, comparing
each test file at its base and head commits:
- all 4 PRs I'd call auto-merge-eligible pass (#48838, #48882, #48945,
#49040)
- 7 of 8 red-flag PRs are flagged (#47698 changes the assertion's
structure, not a key)
- all 3 "right change, wrong key" PRs are flagged, with a fix the model
can apply
- the rule is stricter than maintainers on 5 merged PRs (plain-literal
text edits, broad keys). serge can always meet it by converting to
`Expectations` with the runner key.
The prompt side (the guidance now names the runner key, and a routing
bug kept it from reaching most assertion groups) is in
huggingface/transformers-ci#215.
Tests: `tests/test_expectation_keys.py` (17 cases cut down from the PRs
above) and `ExpectationShapeValidationTests` in `tests/test_tasks.py`.
Full suite: 1390 passed.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Original CI failure
2 integration tests for modelminimaxfailing withoutput_mismatch(tensor values differ (2))tests/models/minimax/test_modeling_minimax.py::MiniMaxIntegrationTest::test_small_model_logits[multi-gpu] (output_mismatch, seen 6/7)CI traceback —
tests/models/minimax/test_modeling_minimax.py::MiniMaxIntegrationTest::test_small_model_logitsWhere to watch it:
Relates to #48514
MiniMaxIntegrationTest::test_small_model_logitsfails on the daily GPU runner because the selected("cuda", 8)expectation drifts from the actual bfloat16 logits by ~0.005 absolute (max ~0.12 relative on a near-zero value), exceedingatol=rtol=1e-3. The divergence is consistent with normal bfloat16 rounding variation, not a model regression. Update the existing CUDA 8 expected slice to the reproduced values; the(None, None)fallback is left unchanged so other hardware keeps passing.Key differences vs. old
("cuda", 8): positions (0,2)-0.3203→-0.3184, (1,0)-0.1201→-0.1177, (1,1)0.4375→0.4355, (1,2)0.2402→0.2393, (2,2)-0.0396→-0.0349; all within ~0.005 absolute.This patch changes only expected values in test files — the assertions were rewritten, not the code under test.
The GPU run below is not evidence that this change is correct: the tests were re-run after the patch rewrote what they assert, so they pass by construction. Judge the new value on its merits.
Possibly related
Existing issues/PRs mentioning
test_small_model_logits(keyword match — not verified to share a root cause):This change was produced automatically by serge from a CI failure report. The patch was generated by an LLM and applied by serge; review before merging.
serge
v0.1.0· model:moonshotai/Kimi-K2.7-Code· 26 LLM turns · 25 tool calls · 27.8s · 468025 in / 4635 out tokens