Skip to content

fix: stabilize nightly verify-test failures (#1256) - #1567

Merged
inureyes merged 3 commits into
mainfrom
fix/issue-1256-verify-test-failures
Sep 1, 2026
Merged

inureyes merged 3 commits into
mainfrom
fix/issue-1256-verify-test-failures

Conversation

@inureyes

@inureyes inureyes commented Sep 1, 2026 •

Copy link
Copy Markdown
Member

Summary

Fix the two known make verify-test failure modes behind issue #1256: the deterministic FAMILY_ORDER drift after adding Inkling, and the intermittent temperature_one_support_unchanged snapshot flake on nightly Metal runs.

What Changed

  • Add the Inkling family to FAMILY_ORDER beside the related LFM2 model block so mlxcel arch remains exhaustive in a deliberate order.
  • Keep sampling::tests::temperature_one_support_unchanged strict on exact support while comparing positive finite snapshot probabilities by ULP budget instead of full bit identity.
  • Bound routed Metal rejection cases to 1 ULP, matching the seven one-ULP differences observed on the failing macOS runners.
  • Bound the non-routed stock-chain case to 2 ULP, matching a focused Linux/CPU observation at one low-probability entry.

Validation

  • cargo fmt --all --check
  • cargo test -p mlxcel-core sampling::tests::temperature_one_support_unchanged -- --exact --nocapture
  • cargo test --bin mlxcel tests::family_order_is_exhaustive -- --exact --nocapture
  • cargo test --bin mlxcel tests::family_order_has_no_orphans -- --exact --nocapture
  • PR CI: formatting, clippy, dependency policy, version, kernel-key, reference, and manifest checks
  • Self-hosted macOS Apple Silicon nightly workflow: fmt, clippy, full make verify-test, and ffmpeg video tests all passed on PR HEAD (run 33505774122)

The full make verify-test target was also attempted on the local Linux/aarch64 host after allowing its MLX FetchContent download. It reached workspace linking, then failed because the macOS-oriented metal,accelerate feature set lacks the MLX symbols mlxcel_set_qmv_wide and mlxcel_qmv_wide on this Linux target. This is a platform gate, not a test failure or macOS execution proof.

Closes #1256

Issue #1256 was failing for two separate reasons on main. `FAMILY_ORDER` omitted the newly added Inkling family, which made `tests::family_order_is_exhaustive` fail deterministically. The `temperature_one_support_unchanged` regression was also too strict for Metal snapshots: the saved support and probabilities remained stable, but nightly runs could drift by a final f32 ULP.

Add Inkling to the CLI family ordering and compare the saved `T == 1.0` sampling snapshot by exact support plus a two-ULP tolerance for positive probabilities. This keeps the intended regression guard while removing the nightly flake.

Validation: `cargo fmt --all --check`; `cargo test -p mlxcel-core sampling::tests::temperature_one_support_unchanged -- --exact --nocapture`; `cargo test --bin mlxcel tests::family_order_is_exhaustive -- --exact --nocapture`; `cargo test --bin mlxcel tests::family_order_has_no_orphans -- --exact --nocapture`.
Follow up the issue #1256 fix with the requested stricter regression boundary. Move Inkling next to the LFM2 family block so the CLI order mirrors the ModelType declaration more closely, and keep the routed `temperature_one_support_unchanged` snapshot at a one-ULP tolerance.

The non-routed stock-chain case still preserves exact support, but it now compares values with a small numeric tolerance because the local CPU backend drifted by two ULP while preserving the same filtered distribution. Validation: `cargo test -p mlxcel-core sampling::tests::temperature_one_support_unchanged -- --exact --nocapture`; `cargo test --bin mlxcel tests::family_order_is_exhaustive -- --exact --nocapture`; `cargo test --bin mlxcel tests::family_order_has_no_orphans -- --exact --nocapture`; `cargo fmt --all --check`.
The previous follow-up loosened the non-routed `temperature_one_support_unchanged` stock-chain case to an absolute epsilon, which was broader than intended. Replace that with the same snapshot helper used by the routed cases: exact support plus a `max_ulp` ceiling.

Local validation on this Linux host showed why the stock-chain case needs `max_ulp = 2`: `cargo test -p mlxcel-core sampling::tests::temperature_one_support_unchanged -- --exact --nocapture` failed at token 12 of the `(top_k=40, top_p=1.0)` case with `0.0016345085` vs `0.0016345083`, a two-ULP drift on a low-probability entry. The routed nightly failure remains capped at one ULP.

Validation: `cargo test -p mlxcel-core sampling::tests::temperature_one_support_unchanged -- --exact --nocapture`; `cargo fmt --all --check`.
@inureyes inureyes added type:bug Bug fixes, error corrections, or issue resolutions priority:high High priority status:review Under review labels Sep 1, 2026
@inureyes
inureyes merged commit e278d01 into main Sep 1, 2026
15 checks passed
@inureyes inureyes added status:done Completed and removed status:review Under review labels Sep 1, 2026
@inureyes
inureyes deleted the fix/issue-1256-verify-test-failures branch September 1, 2026 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority:high High priority status:done Completed type:bug Bug fixes, error corrections, or issue resolutions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[nightly-verify] main is red

1 participant