fix(test): Inkling-VL mixed-prefill scatter test fails intermittently under the full suite and passes in isolation
- Dominant language
- Rust
- Stars
- 467
- Forks
- 54
- Avg merge
- 4h 25m
- Merged PRs (30d)
- 310
Description
## Problem / Background
`vision::inkling_vl::tests::mixed_prefill_scatter_order_is_normalized_text_then_image_then_audio` fails intermittently when the whole root lib suite runs, and passes every time in isolation. The assertion that fails is `assert!(arrays_equal(&actual.inputs_embeds, &expected.inputs_embeds))` at `src/vision/inkling_vl.rs:505`.
## Evidence
Measured 2026-09-04 on Apple M1 Ultra 128GB, macOS 26.6.2, mlxcel 0.7.0-beta.1, release `metal,accelerate`, profile `test-fast`.
One `cargo test --workspace --profile test-fast --features metal,accelerate` run failed with `test result: FAILED. 7722 passed; 1 failed; 131 ignored` on the `-p mlxcel --lib` binary. Re-running only that test three times passed 3 of 3. Re-running the entire `--lib` binary twice passed both times at `7723 passed; 0 failed`. 7722 + 1 equals the 7723 of the clean runs, so it is the same binary and the same test set, and the single failure is not deterministic. Two earlier full-workspace gates on the same `main` lineage the same day passed at 10502 and 10507 tests with zero failures, so this does not reproduce on every run.
## Not caused by the branch it was seen on
It surfaced on `feature/issue-1613-mtp-bench-dispatch`, which changes only `src/bin/speculative_bench.rs`, `src/lib.rs`, `src/server/batch/speculative_burst.rs` and `docs/benchmarks.md`. It touches nothing under `src/vision/`. `src/vision/inkling_vl.rs` was last modified by the Inkling work in #1540, #1546 and #1548.
## Current behavior of the test itself
Two properties of the code under the assertion narrow the search, and both were read out of the tree rather than inferred from the failure.
- **The comparison is exact, not approximate.** `arrays_equal` at `src/vision/inkling_vl.rs:447-451` is `mlxcel_core::allclose(left, right, 0.0, 0.0)`, so rtol and atol are both zero and a single-ULP difference in one element fails the assertion.
- **Both sides of the assertion run the same call sequence.** `expected` is built by the test at `src/vision/inkling_vl.rs:494-501` as `prepare_input_embeddings(input_ids, pixel_values)` followed by `text.merge_audio_embeddings(input_ids, image_first.inputs_embeds, audio_ids)`, and `actual` is `prepare_input_embeddings_with_audio`, whose body at `src/vision/inkling_vl.rs:123-141` is exactly those same two calls on the same model instance with the same input arrays. The test therefore asserts that one call sequence returns bit-identical results across two invocations in a single process, so a plain ordering defect in the scatter would fail deterministically on every run including in isolation, which is not what was observed.
`merge_audio_embeddings` at `src/models/inkling.rs:438-483` runs `tower.forward(audio_input_ids)` and then `crate::vision::merge::merge_llava`, so the audio tower forward executes twice per test run, once for each side of the comparison.
## Why it matters
The assertion compares embedding arrays for a mixed text plus image plus audio prefill scatter. A non-deterministic result there is either test-harness nondeterminism (Rust runs a binary's tests multi-threaded by default, and this repository already has Metal device contention problems between concurrent test binaries) or a real ordering bug in the scatter path that only manifests under load. Those two have very different consequences, and the issue should not be closed by marking the test `#[serial]` until it is established which one it is. Note also that a third outcome is possible and is not benign: if the tower or merge numerics are themselves load-sensitive, that is a production defect in its own right rather than a test artifact, which is the reasoning #1265 applied to the klear parity test.
## Suggested first steps
- Reproduce under load rather than in isolation, for example by looping the full `--lib` binary until a failure is captured, since single-shot observation of a rare event has already produced wrong conclusions in this repository (see #1265).
- Capture the actual and expected arrays on failure instead of asserting a bare boolean, so the diff shows whether the discrepancy is a permutation, a partial write, or numerical. The test module already has `array_to_vec_f32` available for dumping array contents.
- Check whether the scatter path shares any process-global or device state with the other vision tests that run alongside it.
## Scope
**In scope:** classifying the failure, and whatever fix that classification implies in `src/vision/inkling_vl.rs` (the test and the `arrays_equal` helper), `src/models/inkling.rs` (`merge_audio_embeddings` and the audio tower forward), or `src/vision/merge.rs` (`merge_llava`).
**Out of scope:** the granite4_vision and hunyuan_vl flake tracked in #997 and the `mlxcel-core` device-contention failure recorded in #1008. If the root cause turns out to be shared, note it on those issues rather than widening this one.
## Implementation notes
- **Reuse:** extend the existing `arrays_equal` helper at `src/vision/inkling_vl.rs:447` in place if it needs to report a diff, rather than adding a second comparison helper alongside it. The other tests in the module call it too, so an improved failure message benefits them.
- **Constraint:** the workspace has no `serial_test` dependency today, so `#[serial]` is not available without adding a dev-dependency. Do not add one as the fix before the classification step concludes.
- **Constraint:** under `--workspace` cargo runs test binaries one at a time, so a concurrently running `mlxcel-core` binary does not explain a failure inside the root `-p mlxcel --lib` binary. Within that one binary, however, Rust still runs the test functions multi-threaded, which is the contention channel that remains open.
- **Edge case:** the tolerance is exactly `0.0, 0.0`. If the conclusion is numeric jitter, state and justify any tolerance introduced instead of quietly relaxing the comparison until it stops failing.
- **Failure reporting:** on mismatch, report the first differing index and both values, plus the array shapes, so a permutation is distinguishable from a partial write at a glance.
## Acceptance criteria
- [ ] The failure is classified as one of: harness or device contention, load-sensitive numerics in the tower or merge path, or a deterministic defect in the scatter, with the evidence that establishes which recorded on this issue.
- [ ] At least 20 consecutive full `--lib` runs on one commit are executed with each run's exit code and test totals recorded, and the observed failure count is reported.
- [ ] The assertion at `src/vision/inkling_vl.rs:505` reports the first mismatching index, both values, and both shapes on failure instead of a bare boolean.
- [ ] No `#[serial]` attribute, `--test-threads=1` change, or loosened tolerance is merged as the fix before the classification above is complete.
- [ ] If the cause is a real defect in the scatter or merge path, the fix is integrated into the code flow that production prefill actually uses, not confined to the test.
- [ ] `cargo test --workspace --profile test-fast --features metal,accelerate` passes.
## Verification
```bash
# Full gate. Redirect rather than pipe: a piped `| tail` reports tail's exit code and hides the totals.
cargo test --workspace --profile test-fast --features metal,accelerate > /tmp/gate.log 2>&1; echo "exit=$?"
grep -E '^test result:' /tmp/gate.log
# The test alone, which is expected to pass.
cargo test --profile test-fast --features metal,accelerate --lib \
vision::inkling_vl::tests::mixed_prefill_scatter_order_is_normalized_text_then_image_then_audio
# Reproduce under load: loop the whole root lib binary and count failures.
for i in $(seq 1 20); do
cargo test --profile test-fast --features metal,accelerate --lib > "/tmp/lib-run-$i.log" 2>&1 \
|| echo "FAIL on run $i"
done
grep -hE '^test result:' /tmp/lib-run-*.log | sort | uniq -c
```
A pass looks like every run reporting `7723 passed; 0 failed` on the `--lib` binary and a zero failure count from the loop. A reproduction looks like at least one run reporting `7722 passed; 1 failed` with this test named.
## Technical considerations
**Related.** #997 tracks the same class of intermittent failure for `text_only_forward_produces_finite_logits` in the granite4_vision and hunyuan_vl parity tests, and #1008 recorded Metal command-queue failures when two `mlxcel-core` test binaries shared the device. This is a third instance of the family and may share a root cause with them.
**Prior art on the numeric hypothesis.** #1265 established that a parity test in this repository can be nondeterministic run to run from fp32 accumulation order alone, citing #629 and #726 where gpt-oss greedy decode flipped near-tie logits for the same reason, and noted that the verify stream is load-sensitive run to run. That issue was measured on CUDA and this observation is Metal on an M1 Ultra, so it is a hypothesis to test here rather than a conclusion to carry over, but it is the mechanism class worth checking first given the zero-tolerance comparison.
Contributor guide
Research direction
Start with the failing test and arrays_equal in src/vision/inkling_vl.rs, then trace merge_audio_embeddings in src/models/inkling.rs and merge_llava in src/vision/merge.rs. Run the full --lib test repeatedly under the supplied test-fast command and capture mismatch details. Done means the failure is classified with evidence, the assertion reports useful differences, and the full workspace gate passes without masking the cause.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- testing-qa
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100