CI: partition the a2a3 scene corpus and isolate the SDMA fault case - #2485
Merged
ChaoWao merged 2 commits intoSep 30, 2026
Merged
Conversation
|
Warning Review limit reachedNext included review available in 31 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (10)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The three a2a3 pytest selections were not a partition. `sdma_fault` was excluded by name from the demo task but included only by path in the isolated one, and the ordinary sweep excluded `sdma` alone, so the arrangement held only while every `sdma_fault` case also carried `sdma` and lived under `tests/st`. Nothing enforced either condition: a `sdma_fault` case under `examples/` would have run in no task at all, and one carrying `sdma_fault` without `sdma` would have run inside the ordinary sweep on shared devices -- the one place this arrangement exists to keep fault injection out of. All three selections now name `examples tests/st` and decide on both markers: `not sdma and not sdma_fault`, `sdma and not sdma_fault`, and `sdma_fault`. That is disjoint and exhaustive over the four marker combinations, so a case lands in exactly one task however it is marked, and a new fault case needs only the one marker that describes it. `sdma_fault`'s registration states the property that makes it self-sufficient, and says whose teardown hw-native-sys#1425 charges: the case provisions SDMA and then faults an AICore, so the cost falls on the process still holding the 48 STARS streams at that moment. That is why the case gets a task containing nothing else, rather than because a previously provisioned device is expensive to fault on. `docs/ci.md` gains the same correction, and the local-reproduction recipes in `docs/testing.md`, `docs/user/reference/cli.md`, the a2a3 507899 troubleshooting page and the three testing skills move to the sweep's new expression -- a recipe that still deselects `sdma` alone no longer reproduces what CI runs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
The a2a3 scene-test lane ran every
sdma-marked test in onetask-submitjobon two devices, and one of those three tests is a fault-injection case. It now
runs in a job of its own, and the lane's three selections are made a partition.
Run pytest scene tests (a2a3)examples tests/st -m 'not sdma and not sdma_fault'$DEVICE_NUMSDMA pytest (a2a3)examples tests/st -m 'sdma and not sdma_fault'SDMA fault recovery pytest (a2a3)examples tests/st -m sdma_faultA new
sdma_faultmarker carries the selection, registered inconftest.pybeside
sdma.Why the fault case needs a job to itself
Per #1425, provisioning
the PTO-ISA async-SDMA workspace creates 48 device-only STARS streams that sit
in the device fault domain, and once any AICore task has poisoned that card,
tearing those streams down blocks ~306 s — whether the teardown is
rtDeviceResetor an explicitaclrtDestroyStream. Destroying them first doesnot help; the time just moves.
So the cost falls on whichever process still holds those streams when the
fault happens, not on a process that later faults on a previously-provisioned
device. The
sdma_faultcase istest_sdma_worker_aicore_fault_teardown_is_bounded: it provisions SDMA itselfand faults an AICore on purpose, asserting that its own close stays bounded
(
@pytest.mark.timeout(90)). It is #1425's regression test, not a victim ofthe demos.
What sharing a job costs is therefore the other direction: the two SDMA demos
are also
sdma-marked, so their Workers hold 48 streams each, and the existingmarker rule — such a test never shares an L2 Worker, and sorts after every
ordinary test — does not make those Workers gone by the time the fault case
poisons a card. Giving the fault case a session containing nothing else is what
guarantees no other stream holder is alive at that moment.
Why the selections are a partition
Two markers give four combinations, and each must reach exactly one step:
sdmasdma_faultAll three steps name the same corpus (
examples tests/st), so nothing fallsbetween them by living under a path one of them does not walk.
The shape matters because the first commit here was not a partition: the fault
case was excluded by name from the demo task but included only by path in
the isolated one, and the sweep excluded
sdmaalone. That held only whileevery
sdma_faultcase also carriedsdmaand sat undertests/st, andnothing enforced either. A
sdma_faultcase underexamples/would have runin no task; one marked
sdma_faultwithoutsdmawould have run inside theordinary sweep on shared devices, which is the single outcome this arrangement
exists to prevent. Both failure directions were silent —
.claude/rules/ci-change-detection.mdasks every axis to be written in thesame shape for exactly this reason.
Today there is one
sdma_faultcase, it carries both markers, and it lives intests/st, so neither hole was live. The partition is so that adding the nextcase cannot open them.
As it stands
Three tests carry
sdma; one of them also carriessdma_fault. Every steptherefore collects at least one test, so none hits pytest's exit-5-on-empty,
and the fault case runs exactly once rather than in two steps.
Device counts match what each step needs: the fault case is
@pytest.mark.device_count(1)and asks for one; the demos keep two. Thenarrower request is also easier to place in the shared queue.
Docs
docs/ci.mddescribes the partition and carries the #1425 mechanism above,replacing a sentence that said "an AICore fault on a device that has
provisioned SDMA costs minutes instead of milliseconds". The
local-reproduction recipes move with the sweep's new expression —
docs/testing.md,docs/user/reference/cli.md, the a2a3 507899troubleshooting page, and the
testing,test-all-deviceandtest-runtime-deviceskills. A recipe that still deselectssdmaalone nolonger reproduces what CI runs.
Provenance
The first commit is Crane-Liu's, cherry-picked out of #2447 where it arrived
alongside that PR's Qwen qualification work. It is unrelated to Qwen and
touches CI implementation plus shared test infrastructure (
conftest.py),which per
ci-change-detection.mdmust run the full matrix and be verified byobserving what actually ran — evidence a PR reviewed for decode numerics will
not produce. #2447 reverts it, so the two do not both land it.
Verification, and where it stops
Static: the workflow parses as YAML and yields the three selections tabulated
above; the four-combination partition was checked exhaustively rather than by
reading the expressions; the marker is registered;
markdownlint-cli2andruffare clean over every changed file; and a tree-wide grep finds noremaining
-m "not sdma"recipe.pytest --collect-onlywas not run: this worktree has no built_task_interface, soconftest.pycannot import.What CI has to confirm, because nothing else can: that the ordinary sweep
and the demo task each report the tests they should and not the fault case,
and that
SDMA fault recovery pytest (a2a3)reports exactly that one case. Aselection expression that silently matches nothing, or everything, is a green
check either way — so the job logs are the evidence, not the check marks.
Effectiveness is not measured here. No before/after job timing exists for
the split. The previous shared step took two devices, so the demos' Workers may
have held their streams on a card the fault case never poisoned, in which case
the old arrangement was already adequate and this change is preventive. The
cost side is real and worth stating: the lane now makes a second
task-submitsubmission, each with its own
--timeout 1800queue-wait budget.No hardware was taken and no
task-submitlock was acquired. #1425's ~306 s isquoted from that issue, not measured here.
🤖 Generated with Claude Code