[UroojFatima-052]

Closes #1359

Nine tests that crashed part of the windows-latest fleet with 0xc000001d now request aten_half_matmuls instead of skipping, following #1314's trace to CPU bf16/fp16 matmuls. The tenth trains fp32 and just loses its skip.

Changes

• tests/test_rewind_hf.py: the eight real SFTTrainer.train() tests request aten_half_matmuls; skips and the now-unused import removed
• tests/test_v05311.py: test_real_trl_grpotrainer_end_to_end_step requests the fixture; skip and import removed
• tests/test_rewind_wiring.py: skip, its "marker follows the recorder" comment, and the import removed (fp32 run, no fixture needed)
• tests/_windows_ci.py: _windows_ci() docstring now names bitsandbytes only for the two NF4 tests and CPU half-precision matmuls (test(prm): run #1235's CPU bf16/fp16 steps on ATen's matmuls, fixing windows-latest 0xc000001d #1314) for the rest
• tests/test_issue382_windows_ci_guard_is_shared.py:
  • KNOWN_CALL_SITES / GUARDED_TESTS trimmed to test_v07202.py and its two NF4 tests
  • new FIXTURE_TESTS + TestTheHalfPrecisionTestsRequestTheFixture checks the nine tests still request aten_half_matmuls
  • the "find a test by name" lookup moved into _find_test(), shared by both checks

Verification

Local, Windows 11, CPU only, Python 3.11, with CI=true set so the skip would have fired:

• test_rewind_hf.py: 21 passed (all eight previously skipped tests ran)
• test_v05311.py -k test_real_trl_grpotrainer_end_to_end_step: passed
• test_rewind_wiring.py -k test_hf_training_run_records_every_row_once: passed
• test_issue382_windows_ci_guard_is_shared.py: 21 passed
• Removed the fixture from one rewind test as a control: the new guard check failed with the test(prm): run #1235's CPU bf16/fp16 steps on ATen's matmuls, fixing windows-latest 0xc000001d #1314 message, then restored
• ruff check src/soup_cli/ scripts/ tests/ benchmarks/: clean

My CPU likely doesn't hit the fleet's 0xc000001d, so the windows-latest cells are the real check.

Notes

• Tests only, no user-visible change, so no changelog fragment.
• Didn't touch the two @skip_on_windows_ci classes from fix(train): wire resume_from_checkpoint for prm and moe_lora_routing (#1220) #1303 since that PR isn't merged yet. Happy to add them to the check in a follow-up once it lands.

[MakazhanAlpamys]

@UroojFatima-052, welcome, and thank you. This is a clean first PR. It does exactly what #1359 proposed, including the part that usually gets skipped, and the description says precisely what you checked and what you couldn't.

What's good:

• The new FIXTURE_TESTS check is what stops this list from drifting again. I removed aten_half_matmuls from each of the nine tests, one at a time. Every time, exactly one case failed (1 failed / 24 passed), and it named the test.
• Moving the lookup into _find_test() keeps the two checks consistent. If a test is renamed or moved, both checks still fail loudly.
• Leaving test_rewind_wiring.py out of FIXTURE_TESTS, with the reason written next to the list, is right. I measured it: that test runs 840 fp32 SGEMM calls and no half-precision GEMM, so it never needed the fixture.

What I ran on a Windows box (CPU, Python 3.12). The files were test_rewind_hf.py, test_rewind_wiring.py, test_v05311.py, test_issue382_windows_ci_guard_is_shared.py and test_v07202.py::TestTheWindowsCiSkipStaysNarrow:

• Your head with CI=true: 147 passed, 3 skipped, and all ten previously skipped tests ran and passed. The 3 skips are two mlx tests and the guard file's own unskipped-state control.
• Your head merged into current main, CI=true: 147 passed, 3 skipped, all ten passed.
• main with CI=true, for comparison: 138 passed, 13 skipped, with all ten skipped under #382.
• MKL_VERBOSE=1:
  • test_recorded_rows_match_sampler_order goes from 384 GEMM_BF16BF16F32 calls on main to 0 with your change.
  • The GRPO step goes from 28 to 0.
  • So the fixture works in these particular tests too, not only in the ones #1314 measured.
• The narrow-guard checks still hold:
  • Widening _windows_ci() to every Windows box fails 3 tests.
  • Widening it to every CI runner fails 4.
  • Removing the skip from one NF4 test fails the trimmed GUARDED_TESTS check.

Like your machine, this box can't reproduce 0xc000001d. ONEDNN_VERBOSE=1 logs no oneDNN primitive here, so this CPU never takes the code path that crashes the fleet. The three windows-latest cells are the real test. If the crash comes back in one of these tests with the fixture on, #1359's last criterion applies: only that test gets its skip back, with the log linked.

Three optional suggestions. None of them blocks the merge:

1. A skip added back to one of the nine still passes the guard. I put @skip_on_windows_ci back on test_recorded_rows_match_sampler_order and kept the fixture: 25 passed. Adding one assert to test_the_test_requests_the_fixture closes that gap. It also forces the path the issue describes: move the test to GUARDED_TESTS and link the log.

           assert "skip_on_windows_ci" not in _decorator_names(node), (
               f"{filename}::{target} requests aten_half_matmuls and must not also skip; "
               f"if it needs the #382 skip back, move it to GUARDED_TESTS and link the log"
           )

   With it, the same mutation fails (1 failed), and the unmutated run stays at 25 passed.
2. tests/test_rewind_hf.py:279-281: the arguments are indented 8 spaces and the closing ): 4. The same file uses the usual form at :308-310: arguments at 4 spaces, ): at column 0.
3. tests/_windows_ci.py:3-5: the module docstring still says the guard is "Imported by every test file that reaches a real trainer.train()" and "needed in four of them". After this PR only test_v07202.py imports it.

On #1303: thanks for flagging it. When it lands, nothing in the guard will fail, because KNOWN_CALL_SITES isn't compared with the files that actually import the marker. So a follow-up after it merges, as you suggested, is the right place.

This is ready to merge. It now waits only on the required checks, which are still queued because the shared runners are busy. If you push again (for example to add suggestion 1), the new run needs a maintainer's approval before it starts, since this is your first contribution, and it joins the back of the queue. So it's also fine to leave the suggestions for a follow-up.

[UroojFatima-052]

Thanks @MakazhanAlpamys for the review. Since CI hadn't started yet it was cleaner to fix those suggestions in this PR.

Applied all three suggestions. Looking forward to the merge and to contributing more here.

[MakazhanAlpamys]

@UroojFatima-052, thank you for taking the three suggestions, and for doing it while the first run was still waiting. I went through the new push (f02f4dbe) and checked it against the code, not just the description.

What is good

• The skip-plus-fixture assertion works. I put @skip_on_windows_ci back on test_recorded_rows_match_sampler_order and kept aten_half_matmuls: the new assert fails it (1 failed, 20 passed), and the message tells the next person to move the test to GUARDED_TESTS and link the log. Unmutated, the guard file and the narrow-skip class are 25 passed.
• The docstrings now say what the code does. tests/_windows_ci.py names bitsandbytes only for the two NF4 tests and says the other tests request the fixture, which matches the one remaining import of the marker (test_v07202.py).

What I ran (CPU, CUDA_VISIBLE_DEVICES=-1, CI=true so the skip would fire, your head merged into current main, which merged cleanly):

• test_issue382_windows_ci_guard_is_shared.py, TestTheWindowsCiSkipStaysNarrow, test_rewind_hf.py, test_rewind_wiring.py, test_v05311.py: 147 passed, 3 skipped. The 3 skips are two mlx tests and the guard file's own unskipped-state control. All ten tests that used to skip ran and passed.
• Merged again with the newest main and re-ran the same targets with only the GRPO test from test_v05311.py: 59 passed, 3 skipped. ruff check is clean.
• The new guard file on clean main: 9 failed, 12 passed. The 9 are the TestTheHalfPrecisionTestsRequestTheFixture cases, since main still skips those nine tests.
• Eight mutations:
  • Six fail a test:
    • dropping the fixture from a rewind test (1 failed);
    • dropping the fixture from the GRPO test (1);
    • re-adding the skip next to the fixture (1);
    • removing the skip from an NF4 test (1);
    • widening _windows_ci() to every Windows box (3);
    • widening it to every CI runner (4).
  • Making aten_half_matmuls a no-op passes your guard file. That is expected, because your file checks that the tests request the fixture, not what the fixture does. The existing TestTheAtenMatmulSwitchStaysScoped::test_the_fixture_switches_both_off in test_issue1235_prm_fp32_master_weights.py catches it (1 failed).
  • Re-adding a skip to test_hf_training_run_records_every_row_once passes, because that fp32 test is in neither list. That was decided last round and I am not reopening it.
• Your earlier head on windows-latest: the required run on 2d8d62ca was green on all three windows cells. In the 3.12 and 3.10 logs, all ten tests show PASSED, the two NF4 tests stay SKIPPED, and there is no 0xc000001d anywhere. That is good evidence for the approach, but it was measured on the hosts those jobs happened to get. The windows cells on this head are still the real check.

Two cosmetic leftovers. Neither blocks the merge, and there is no need to push again for them:

1. tests/test_rewind_hf.py:281: the arguments are at 4 spaces now, but the closing ): is still at 4. Your own file uses the usual form at :308-310, with ): at column 0.
2. tests/test_issue382_windows_ci_guard_is_shared.py:200-202: the blank line after the new assert block went away, so one blank line precedes @pytest.mark.skipif( where PEP 8 wants two. ruff check does not flag it, and this repo keeps ruff format off.

Status. Nothing in the code or the tests needs changing; it is ready to merge. The only thing it waits on is the required checks on this head. As your first contribution, a maintainer has to approve the runs for this push before they start.

One note for the record: if 0xc000001d comes back in one of these ten tests on a windows-latest cell, issue #1359's last criterion applies. Only that test gets its skip back, with the log linked.

[MakazhanAlpamys]

Merged as a7a26fc7. Thank you, @UroojFatima-052: you did the part of this issue that usually gets skipped, the FIXTURE_TESTS check, so the list cannot drift again. Removing the fixture from any of the nine now fails a test that names it, and you added the assert that also fails a test carrying both the fixture and the skip. The description said exactly what you ran and what your machine could not show. I ran the touched files with CI=true: 147 passed, 3 skipped. Of 8 mutations, 6 fail a named test, 1 is caught by an existing test elsewhere, and 1 is a documented gap. The new guard file fails 9 on main.