Add: AICore fault triage to the device error code guide - #1501
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesDevice error documentation
Estimated code review effort: 1 (Trivial) | ~5 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 two red The failing step is "Run Python unit tests", and the failure is: That is my test from #1495, already on Fixed in #1504 (dedicated pid file instead of parsing a log stream). This PR will go green on a rebase once that lands; nothing to change here. |
e619503 to
439325c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/troubleshooting/device-error-codes.md`:
- Around line 275-281: Revise the guidance in the out-of-range UB address
section so compile-time TASSIGN offsets and template-derived extents are ruled
out only after calculating their maximum reachable byte bound and confirming it
fits within the AIV UB size for every admitted input. Remove the unconditional
“cannot” assertion, and direct readers to F1’s second case only when that bounds
comparison succeeds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d11567c-7ecf-4ec0-9f10-0339a784955d
📒 Files selected for processing (1)
docs/troubleshooting/device-error-codes.md
439325c to
53e09bf
Compare
|
Updated now that #1489 is closed. Still worth merging — the content was never specific to that issue. The gap this fills is structural: the guide had "chasing down a capacity code" and "chasing down a stall", but nothing for an AICore addressing fault, which is the third distinct way a run reaches the same generic The worked example got stronger, not weaker. #1489 closed as very likely fixed by #1477 — a The section now says that outcome explicitly. A worked example that names where it ended up is worth more than one that trails off at "and then we could not reproduce it". Everything else stands unchanged, in particular the note that the harness will not isolate your device log — that is why #1489's one real occurrence lost the artifact that would have decided F1 on the spot, and it is still true for the next one. |
53e09bf to
3e38c65
Compare
The guide covered capacity codes and stalls but had nothing for an AICore addressing fault, which is the third way a run reaches the same generic 507018. hw-native-sys#1489 spent an investigation re-deriving the procedure from scratch and still could not close, so record it. Three steps, ordered so the cheap disqualifications come first: - F1 separates "the kernel computed a bad address" from "the core was made to execute something that is not the kernel". `binSize` in GetBinAndKernelNameExceptionArgs matching the runtime's own aicore_kernel.o means the report is naming the polling-dispatch executor, so the fault is a dispatch-payload problem and no amount of reading the kernel's arithmetic will find it. hw-native-sys#1036 is the worked example. - F2 rules the kernel in or out statically. Constant TASSIGN bases plus template tile extents do not make an out-of-range address impossible — the constants can overrun on their own — but they make it decidable on paper, because runtime values that only shrink a tile move no base and grow no extent. Summing the highest byte reached and comparing against the UB size settles it either way without touching the device. This is what cleared the allreduce collectives in hw-native-sys#1489. - F3 separates a real fault from a post-mortem register dump of a core reaped mid-spin, by counting which detector actually fired. hw-native-sys#1489 has since been closed as very likely fixed by hw-native-sys#1477 — a CoreTracker whose uint64_t state overran past 21 clusters while those cases ran at the device's full 24. F2's verdict held: the kernels were never at fault, and the answer was in F1's second case all along. The section says so, because a worked example that names its own outcome is worth more than one that trails off. Also state where the device log has to be redirected and why the harness will not do it: outputs/<case>_<ts>/ exists only when a DFX flag is on, so a plain onboard run leaves its log in the shared ~/ascend/log/debug/ with every other user's, and the evidence is unattributable once the run ends. That is how hw-native-sys#1489's only real occurrence lost the one artifact that would have decided F1. Renumber the trailing section and fix the in-page link that pointed at its old number. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… dots `fix-pr` Step 6 told you to squash first and rebase after: the `> 1` row soft-resets to `$BASE_REF`, and only the following list says "rebase onto `$BASE_REF`". `commit-and-push` has the opposite (correct) order, so the two documents disagreed and the skill's order is the one that corrupts the commit. `git reset --soft "$BASE_REF"` moves HEAD to the base but keeps the branch's index. If the branch still sits on an older base — which it does whenever anything merged while the PR was open — every file the base gained meanwhile is recorded as a deletion by the PR. Rebasing afterwards does not repair it: the revert is by then part of the PR's own diff and replays cleanly onto the new base. It surfaces as unrelated files being reverted, easy to miss in a large diff and easy to merge. Hit while squashing hw-native-sys#1506: `docs/troubleshooting/device-error-codes.md` came back as -86 lines because hw-native-sys#1501 had merged since the branch point. Step 6 now fetches and rebases at the top, *before* the edit step rather than after it: git refuses to rebase a dirty worktree, so a rebase placed after the fix cannot run at all. Verification gains a three-dot diff of the file list, and it lives with the single-commit check after the commit exists — at the soft-reset itself `HEAD` is `$BASE_REF`, so that diff is empty by construction and would have verified nothing. Two dots would also list files the base has and the branch does not, rendering them as the branch's deletions: it both hides a real revert in the noise and invents fake ones. The single-commit check now separates its outcomes, since the surrounding table already treats them differently: `> 1` squashes again, `0` stops. `commit-and-push` §1 gains the fetch, and its soft-reset step states the precondition it silently relied on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… dots (#1516) `fix-pr` Step 6 told you to squash first and rebase after: the `> 1` row soft-resets to `$BASE_REF`, and only the following list says "rebase onto `$BASE_REF`". `commit-and-push` has the opposite (correct) order, so the two documents disagreed and the skill's order is the one that corrupts the commit. `git reset --soft "$BASE_REF"` moves HEAD to the base but keeps the branch's index. If the branch still sits on an older base — which it does whenever anything merged while the PR was open — every file the base gained meanwhile is recorded as a deletion by the PR. Rebasing afterwards does not repair it: the revert is by then part of the PR's own diff and replays cleanly onto the new base. It surfaces as unrelated files being reverted, easy to miss in a large diff and easy to merge. Hit while squashing #1506: `docs/troubleshooting/device-error-codes.md` came back as -86 lines because #1501 had merged since the branch point. Step 6 now fetches and rebases at the top, *before* the edit step rather than after it: git refuses to rebase a dirty worktree, so a rebase placed after the fix cannot run at all. Verification gains a three-dot diff of the file list, and it lives with the single-commit check after the commit exists — at the soft-reset itself `HEAD` is `$BASE_REF`, so that diff is empty by construction and would have verified nothing. Two dots would also list files the base has and the branch does not, rendering them as the branch's deletions: it both hides a real revert in the noise and invents fake ones. The single-commit check now separates its outcomes, since the surrounding table already treats them differently: `> 1` squashes again, `0` stops. `commit-and-push` §1 gains the fetch, and its soft-reset step states the precondition it silently relied on.
Why
docs/troubleshooting/device-error-codes.mdcovers capacity codes (§2) and stalls (§3), but an AICore addressing fault is the third way a run reaches the same generic507018— and it had no section. #1489 re-derived the procedure from scratch over a full investigation and still could not close, so this records what that investigation established.What
A new §4 with three steps, ordered so the cheap disqualifications come first:
F1 — is it the kernel's addressing, or did the core jump elsewhere? These look identical from the host and have nothing in common as bugs. If
GetBinAndKernelNameExceptionArgs: binSizematches the runtime's ownaicore_kernel.orather than your kernel, the report is naming the polling-dispatch executor and the fault is a dispatch-payload /function_bin_addrproblem — no amount of reading kernel arithmetic will find it. #1036 is the worked example.F2 — rule the kernel in or out statically, before instrumenting. A kernel whose
TASSIGNbases are compile-time constants and whose extents come from a template cannot compute an out-of-range UB address; runtime values that only shrink a tile move nothing. Summing the highest byte reached against the UB size settles it on paper. This is what cleared all five allreduce modes in #1489 (constant bases,nranksonly divides the chunk count, ≤132 KiB of 192 KiB at every rank count).F3 — real fault vs post-mortem register dump. A core reaped mid-spin can produce a fault-looking line describing the state it died in. If the deadlock and SPIN detectors are both zero and only
HandleTaskTimeoutfired, nothing on the device proved a fault — the wait that never completed is the investigation, not the addressing line.The part that matters most
The section states plainly that the harness will not isolate your device log:
outputs/<case>_<ts>/is created only when a DFX flag is on, so a plain onboard run leaves its log in the shared~/ascend/log/debug/among every other user's on the box.That is not hypothetical — it is why #1489 is still open. Its one real occurrence produced exactly the artifact that would have decided F1, and it was unattributable by the time anyone looked.
.claude/rules/running-onboard.mdalready says to exportASCEND_PROCESS_LOG_PATH; what was missing is what to read once you have the log, which is the substance here.Notes for review
doc-consistency.md§6 — it was already over before this change. The natural split is a landing page plusdevice-error-codes/{capacity,stall,aicore-fault,untested}.md. I did not bundle that restructure into a content-only change; flagging it instead, per that rule's "flag it to the user" clause.🤖 Generated with Claude Code