ci: name the kernel behind a GPU memory fault, and stop CI condemning healthy nodes - #1800
Merged
Conversation
A GPU memory fault reaches CI as an address and nothing else: Memory access fault by GPU node-9 (Agent handle: 0x...) on address 0x... Measured on a Frontier compute node with MFC's own module set, the offload runtimes will say considerably more than that. Under CCE, CRAY_ACC_DEBUG=1 names the kernel and the source line of the launch that faulted: ACC: Execute kernel fault_$ck_L8_1 async(auto) from fault.f90:8 Memory access fault by GPU node-4 ... Under the AFAR toolchain frontier_amd uses, OFFLOAD_TRACK_ALLOCATION_TRACES states whether the address ever belonged to a host-issued allocation, which separates an out-of-bounds write from an unmapped one. Neither can be on for a whole run: CRAY_ACC_DEBUG prints per kernel launch and per transfer, and MFC launches thousands per timestep. So spend a retry on it. MFC already retries a failed case up to three times, and those retries rescue almost nothing -- 0 of 235 in bench, with every recorded failed test showing the full attempt count. That last fact is what makes this work: when a case fails it fails all its attempts, so the retry is a reproduction of the fault that has already been paid for and currently produces nothing. On a GPU memory fault the next attempt now re-runs with both variables set. Both, rather than detecting the cluster: each runtime ignores the other's, verified on both toolchains. Nothing changes for any other failure, and nothing changes on the happy path. Not placed in the .mako templates. Those generate job scripts for every ./mfc.sh run on all 18 supported clusters, so anything set there would follow users into production runs. The environment is built per subprocess in the test harness instead -- also the reason it is a fresh dict rather than os.environ, since cases run in worker threads and a mutated global would leak per-kernel logging into every concurrent case. Measured while establishing the above, on the same AFAR drop Frontier uses: allocation tracking costs 10.5x on a loop that maps and unmaps every iteration, and nothing measurable on MFC's shape (map once, then kernels and target updates: 2.606s -> 2.620s over 2000 iterations). Also learned and deliberately not acted on: GPU core dumps do land on Frontier when the working directory is node-local, but a single faulting run wrote 1.1 GB of core plus 15 gpucore files of ~157 MB each. The CI failure "GPU core dump failed / Failed to allocate file: Bad file descriptor" is the runner workspace being on Lustre, and it is accidentally protective. 510 tests pass.
Measured on Frontier: CRAY_ACC_DEBUG=1 emits 142,777 "ACC:" lines for a
single 800-cell 1D case, one per kernel launch and per transfer. The
previous commit echoed a failing attempt's output whole, so a diagnostic
retry would have buried the failure it exists to explain under six
figures of runtime chatter.
Only the tail is worth keeping. The fault comes last, and the launch
immediately before it is what names the kernel and source line:
ACC: Execute kernel syscheck_$ck_L89_1 from .../syscheck.fpp:89
Memory access fault by GPU node-4 ...
Ordinary failures still print in full -- they are short and the whole
thing is useful. Only the diagnostic retry is capped, and the complete
capture remains in out_pre_sim.txt for anyone who wants it.
The same session settled the two things this design rested on:
chain the variable does reach the binary through ./mfc.sh run ->
frontier.mako -> srun -> binary, so the change is live, not inert
cost 13.7s -> 17.3s (1.27x) on the case that produced those 142,777
lines. Against the 1 hour TEST_TIMEOUT_SECONDS a case would need
to take ~2800s unaided before a diagnostic retry could push it
over, and the slowest case seen in CI is around 1000s. So the
retry cannot convert a fault into a timeout, which would have
hidden the very thing it is meant to surface.
The logging is per case and per retry -- case_env is local to
handle_case and only set once that case has faulted -- so a suite with no
GPU faults is bit-for-bit unaffected, and one with a fault pays 1.27x on
exactly one case.
511 tests pass.
A deliberate out-of-bounds device write in the RK update, so CI produces a real GPU memory fault and the retry diagnostics from the preceding two commits can be seen end to end in a job log rather than argued about. Scoped to the igr branch, which a handful of tests exercise, rather than firing for every GPU case and burning the whole matrix. Not in syscheck: a faulting syscheck would trip the preflight, which would then start excluding perfectly healthy nodes. Expected in the log of a Frontier or Phoenix GPU leg: <case> failed, Memory access fault by GPU node-N ... GPU memory fault: retrying <case> with offload diagnostics enabled ACC: Execute kernel <name> from src/simulation/m_time_steppers.fpp:<line> Memory access fault by GPU node-N ... Revert this commit before the PR is considered for merge. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
|
Claude Code Review Head SHA: 3ef4c28 Files changed:
Findings:
|
sbryngelson
marked this pull request as ready for review
September 2, 2026 01:42
sbryngelson
changed the title
GPU_PARALLEL_LOOP expands to nothing on CPU builds, so the deliberate out-of-bounds write was also executing on the host in every CPU igr test. That is undefined behaviour rather than the clean device fault this is meant to produce, and it would have made the CPU legs fail for a reason unrelated to what is being demonstrated. Gated on MFC_GPU so only the GPU legs fault. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The diagnostic retry currently won’t trigger because the retry loop checks the wrong text for the GPU-fault signature, and the PR also contains an intentional out-of-bounds device write that must be removed before merge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This draft PR aims to make GPU memory faults actionable in CI by using the test harness’ existing retry mechanism: when a run fails with a GPU memory fault, the next attempt re-runs with offload-runtime diagnostics enabled and prints only the bounded tail of the output.
Changes:
- Thread a per-subprocess
envthrough the test harness so retries can enable offload diagnostics without mutating globalos.environ. - Detect GPU memory fault signatures and (intended to) trigger a diagnostic retry that prints only the last N lines to avoid log flooding.
- Add unit tests for the GPU-fault detection/diagnostic-env logic, and (intentionally, per PR description) inject an out-of-bounds device write to force a real GPU fault.
File summaries
| File | Description |
|---|---|
| toolchain/mfc/test/test.py | Adds GPU-fault detection + diagnostic retry env plumbing and bounds CI log output on diagnostic retries. |
| toolchain/mfc/test/case.py | Allows passing a per-subprocess environment to mfc.sh run via common.system(..., env=...). |
| toolchain/mfc/test/test_gpu_fault_diagnostics.py | Adds unit tests covering GPU fault signature matching and diagnostic env composition/usage. |
| src/simulation/m_time_steppers.fpp | Injects a deliberate out-of-bounds device write to force a GPU memory fault (must be removed before merge). |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The first attempt used j + 1e8 (762 MB past the array base) and produced no fault at all: the Frontier benchmark leg ran the igr case five times and passed. Reproduced on an MI210 with the same AFAR toolchain. The offset has to clear MFC's *whole* device footprint, not one array: 32 MB allocated, offset 1e8 -> faults (why the first local test misled me) 4 GB allocated, offset 1e8 -> NO fault (lands in the next allocation) 4 GB allocated, offset 2e9 -> memory access fault 4 GB allocated, offset 1e11 -> HSA_STATUS_ERROR_MEMORY_APERTURE_VIOLATION So there is a window. Too small and it silently corrupts a neighbouring array; too large and the runtime reports an aperture violation, which is a different message that is_gpu_memory_fault does not match and which would not exercise the diagnostic path either. 2e9 elements is 16 GB, clear of MFC's allocations and still an ordinary memory access fault. It also stays inside a default 4-byte integer. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
The failure site raised "[gpu-memory-fault]" while the retry searched for "memory access fault by gpu", so the two never matched and the diagnostic could not fire. CI proved it: a Frontier gpu-omp shard hit 216 memory access faults and enabled diagnostics zero times. The marker is now one of the signatures verbatim, and parenthesised rather than bracketed -- Rich parses "[...]" as a style tag and deletes it, which is why that shard logged a bare "Failed to execute MFC. " with the marker missing. The three tests this replaces asserted only that the source text contained certain identifiers, which cannot detect a mismatch between the string one side writes and the string the other side reads. The two new tests exercise the hand-off and the Rich rendering; both are verified red against the respective bugs. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
The diagnostic fired correctly on Frontier CCE (39 retries, 4413 CRAY_ACC_DEBUG lines) and then pointed at the wrong kernel. With a known out-of-bounds write injected into m_time_steppers, the last kernel logged before each fault was s_write_run_time_information in 111 of 140 faults, s_igr_riemann_solver in 23, and m_time_steppers in none. Dispatches are asynchronous, so the fault is reported long after the launch that caused it and the trace's tail is whatever ran next. A trace that confidently accuses the wrong kernel is worse than no trace, so the retry now sets AMD_SERIALIZE_KERNEL/COPY=3. Also corrects the docstring claim that CRAY_ACC_DEBUG=1 names the launch that faulted; this run falsified it. Whether CCE's offload runtime honours the HIP serialization vars is what the next run measures. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
Measured on the CCE gpu-acc shard at 45ec609: with AMD_SERIALIZE_KERNEL/COPY=3 set, the same injected fault still blamed s_write_run_time_information in 168 of 213 faults and m_time_steppers in none -- the distribution is unchanged from before serialization. They are HIP runtime variables; CCE's offload runtime is not HIP. This also removes a claim I had no measurement for: the previous docstring said serialization was "verified to be honoured on the AFAR/HIP path". It was inferred from the variables being HIP's, not measured. The AMD lanes are still pending. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
The fault-injection experiment measured what each variable was worth, and most were worth nothing. AFAR/OpenMP already names the faulting kernel unaided -- "Kernel 0: omp target in _QMm_time_steppersPs_tvd_rk @ 486", correct in 90 of 90 faults and printed on the FIRST attempt, before any diagnostic is enabled. That falsifies the premise this retry was built on, that a fault reports only an address. CCE/OpenACC cannot name it at all: across three runs CRAY_ACC_DEBUG blamed s_write_run_time_information 386 times and the true culprit 0 of 473, because dispatch is asynchronous and its log's tail is whatever ran next. AMD_SERIALIZE_* changed neither lane. Both are removed -- a confidently wrong suspect is worse than no diagnostic. OFFLOAD_TRACK_ALLOCATION_TRACES stays: it reports whether the faulting address was ever a real host allocation (60 retried faults, 0 unretried), which the runtime does not volunteer. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1800 +/- ##
=======================================
Coverage 62.26% 62.26%
=======================================
Files 84 84
Lines 21558 21558
Branches 3188 3195 +7
=======================================
Hits 13423 13423
Misses 5937 5937
Partials 2198 2198 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The diagnostics only emit when the runtime is already aborting on a memory fault, so they are inert in a healthy run and there is nothing to save by withholding them. Setting them on every run makes attempt 1 carry the evidence, which is what the retry existed to obtain. Adds OFFLOAD_TRACK_NUM_KERNEL_LAUNCH_TRACES=8 -- host stack traces for recent launches, which the runtime advertises in its own fault message -- alongside the allocation verdict. Adds NVHPC's wording to the fault signatures. It says "Accelerator Fatal Error / CUDA_ERROR_ILLEGAL_ADDRESS", nothing like AMD's "memory access fault by GPU", so 189 faults on a Phoenix gpu-acc shard were never recognised as GPU faults. Still not setting CRAY_ACC_DEBUG: it streams a line per launch for the whole run and, because CCE dispatches async by default (acc_model=auto_async_kernel), its tail names whatever ran next -- the wrong kernel in 81 of 102 traced faults. The flag that would fix that, -h acc_model=auto_async_none, is a compile flag no retry can set. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
…tion CCE defaults to acc_model=auto_async_kernel, so a memory fault surfaces at an unrelated sync point and its trace names the wrong kernel (81 of 102 traced faults blamed s_write_run_time_information, 0 named the culprit). auto_async_none executes kernels synchronously, which should make the abort land on the faulting kernel. Scoped to Cray + OpenACC: acc_model is an OpenACC flag, so the OpenMP offload builds are unaffected by construction. This is a measurement, not a proposal. Even if it works it should probably not ship in CI builds: it would stop the test suite exercising the asynchronous dispatch that production runs use. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
… attribution" This reverts commit 7e21c29.
Removes the three DO NOT MERGE commits' effect on m_time_steppers.fpp (3ef4c28, cc4b52e, c2d0579). The file is now byte-identical to master. The injection did its job: it is the only reason the diagnostics could be checked against a known ground truth, which is how the original design was found to be measuring the wrong thing on every lane. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
1. The detection path had no effect. is_gpu_memory_fault tagged the exception and nothing read the tag: classify_error bucketed anything containing "failed to execute" as a generic execution failure, so a GPU memory fault -- the one execution failure a retry provably cannot fix -- was indistinguishable from a transient launcher problem. It now gets its own bucket, which is what the detection was kept for. 2. "accelerator fatal error" was too broad. NVHPC uses that prefix for unrelated failures, including "call to cuMemAlloc returned error 2: Out of memory"; classifying an OOM as a memory fault would send the reader hunting a bad index that does not exist. cuda_error_illegal_address already matches the real thing. 3. Restart cases bypassed the diagnostics entirely -- run_restart never took an env, so a fault there produced none of the output this exists to provide. 4/5. Comments still described the retry that was removed, and one clause did not parse. Findings 1 and 4 were both residue from deleting the retry: the mechanism went, its vocabulary stayed. The tests missed it because they asserted the marker round-trips, not that anything consumes it. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
Measured on Frontier (CCE 19.0.0, ROCm 6.3.1): HSA_TOOLS_LIB=librocm-debug-agent.so.2 prints "Disassembly for function s_tvd_rk$m_time_steppers_$ck_L486_6" -- subroutine, module and source line of the injected fault -- plus the faulting instruction and per-wave registers, straight to the job log. The earlier conclusion looked only at CCE's own trace and generalised from it to the machine. The information was available one layer down, at ROCr. Env-only: no recompile, no execution-model change. Not enabled yet: its cost on a healthy run is being measured, and that decides always-on versus a documented recipe. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
HSA_TOOLS_LIB=librocm-debug-agent.so.2 is the only thing that names a faulting kernel on CCE. Measured on Frontier (CCE 19.0.0, ROCm 6.3.1): it prints "Disassembly for function s_tvd_rk$m_time_steppers_$ck_L486_6" -- the exact injected fault site -- with the faulting instruction and per-wave registers, where no CRAY_ACC_* variable names it at all. Enabled wherever the library is reachable. The gate is evaluated per call, never at import. On Frontier the library is on disk the whole time but only reaches LD_LIBRARY_PATH once mfc.sh load runs, so an import-time gate reports "absent" on the one machine this is for, indistinguishably from Phoenix where it truly is missing. It probes for the file rather than dlopen'ing it, so testing the subprocess's environment does not load a debug agent into the harness. The agent emits ~14k lines per fault, almost all of it one disassembly and register dump repeated per wave. summarize_rocm_debug_agent collapses that to ~37 lines. A fixed tail cannot substitute: on the real report the first 80 lines are one wave's registers and the last 80 another's, and the kernel name appears in neither. The stop-PC histogram is kept because the modal PC was a load while the fault is a write, so a single PC would name the wrong instruction. Cost on a healthy run: 4.5645 ns/gp/eq/rhs against an agent-free spread of 4.5301-4.5614 -- 0.07% above a range 0.69% wide. That is n=1 by decision, not by measurement, and the comment says so. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
Every command in the investigation was --gpu acc. The OpenMP-offload lane was never built or run, so listing it as working was an inference sitting in a table of measurements. The agent hooks ROCr, below both OpenACC and OpenMP offload, so it should fire either way -- but the claim is attribution, not firing. s_tvd_rk$m_time_steppers_$ck_L486_6 is CCE's OpenACC symbol mangling, and whether module, subroutine and line survive in the OpenMP-offload form is unverified. The summarizer is unaffected: its regex takes whatever the symbol is. Also upgrades the fixture to the real report's format -- the "(Agent handle: ...)" clause, "End of disassembly." as terminator, and the blank line plus "scalar registers:" header before a wave's dump -- and adds a test pinning the field set the summarizer produced from the genuine 14,635-line log. Structure only: pinning the wave counts or PC histogram would encode one fault instead of testing the code. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
The regexes were written against ROCm 6.3.1 and silently produced '' for 65,210 lines of real 7.2.0 output -- on the AFAR lane, the very one they were meant to serve, with no error to explain it. Two format changes: wave line: 7.2.0 inserts kernel_code_entry= and kernargs= BETWEEN the pc and "(stopped, reason:", which the adjacency-requiring regex rejected. fault line: "OFFLOAD ERROR: memory access fault ... at virtual address ... Reasons:" instead of "Memory access fault ... on address ... Reason:", and the lookup was case-sensitive on "Memory". Fixed at three sites; the fault line now reuses is_gpu_memory_fault, which already knows every wording, instead of hardcoding one version's. Both formats are pinned by fixtures built from real reports, and neither may be fixed at the other's expense. This is the failure a single-version fixture cannot catch: it passes while the lane produces nothing. Found only because someone ran it against the real file. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
CCE OpenMP closes the last cell: s_tvd_rk$m_time_steppers_$ck_L486_16, the same scheme as the OpenACC lane's _ck_L486_6 and differing only in a trailing counter. That overturns the assumption behind the previous comment. The symbol form is set by the COMPILER, not the offload model: CCE emits its own scheme for both acc and mp, while AFAR's Flang form (__omp_offloading_..._QMm_time_steppersPs_tvd_rk_l486) is different again. Reading any two lanes suggests the offload model decides; only all three show otherwise. All carry module, subroutine and line. Also records that the summarizer is now validated against three real reports (14,635/13,826/65,210 lines in, 37/35/36 out) rather than one, and that the stop-PC histogram earns its place most on CCE OpenMP, which halts at seven distinct PCs against four for CCE OpenACC and one for AFAR. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
… summary is missing Two gaps closed. 1. Silent degradation is now loud. When the debug agent is reachable and the failure IS a GPU memory fault but no agent report is recognised, that is either a failed load or a format change -- and until now the only symptom was raw output where a summary should have been. That is exactly how a ROCm 6.3.1-only parser sat on the AFAR lane returning nothing for 65,210 lines. It now says so. 2. bench.py and run_case_optimization.sh had no fault handling at all. Both run GPU cases; neither set the diagnostics, and bench printed a fixed log_tail on failure, which cannot surface an agent report -- on a real one the tail is a single wave's registers and the kernel name is not in it. The diagnostics move to mfc/gpu_diagnostics.py now that three callers share them; bench.py depending on the test module to explain a crash would be the wrong way round. .github/scripts/summarize_gpu_fault.py gives the shell script the same summary, exiting 1 when there is no agent report so the caller falls back. The bench test needed padding past log_tail's 60-line window: with a 20-line fixture the tail contains the kernel name and the test passes against the old behaviour, proving nothing. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
mfc.sh test and mfc.sh bench are developer commands, not only CI entry points, and the agent was enabled purely on the library being reachable -- so it switched on for local runs on any ROCm machine. mfc.sh run is untouched and unaffected. Two ways that was wrong, both silent. Setting HSA_TOOLS_LIB behind someone collecting a GPU core dump gives them "Failed to enable debug interface" and no dump, because the agent and ROCr core dumps are mutually exclusive -- the same path an attached rocgdb trips. And the OFFLOAD_TRACK_* values overwrote whatever the caller had chosen. An explicit setting is now authoritative: the agent is skipped when HSA_TOOLS_LIB or HSA_ENABLE_DEBUG is already set, and the other two are defaults rather than overrides. Same rule in the case-optimization script. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
Frontier CCE --gpu mp, ROCm 6.3.1, agent 2.0.3, four interleaved pairs, all twelve runs valid. Healthy run: no effect detected. The agent's whole range sits inside the agent-free range, paired differences split 2 up / 2 down, mean -0.045%. Resolution is ~0.8% set by the agent-free spread, so this is "no effect detected at n=4", not "no effect". Healthy-run log noise is zero -- 2661-2662 bytes with and without. Faulting run: +0.387 s, 1.60x of a 0.647 s baseline, which is 0.011% of the 1-hour test timeout. A fault cannot become a timeout through the agent -- the risk worth checking, since a diagnostic that hides the fault it explains is worse than none. The cost that is real is volume: 6.7 MB / ~13,630 lines per faulting test on that lane, ~65,000 on AFAR. That makes the summarizer load-bearing rather than an optimisation. Also replaces the AFAR-interaction caveat with the measurement that settled it: the agent does not supersede libomptarget (OFFLOAD ERROR 1, Libomptarget 8, identical with and without); it is mutually exclusive with ROCr core dumps only. Timings are CCE only; the AFAR lane produces twice the waves and was not re-timed. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
OFFLOAD_TRACK_ALLOCATION_TRACES and OFFLOAD_TRACK_NUM_KERNEL_LAUNCH_TRACES were set on every run. They instrument every allocation and every kernel launch, so a healthy run pays continuously. Measured on an MI210 with amdflang/libomptarget, test AFBCBDFA: neither 5.94 s passes allocation traces only >400 s timed out launch traces only >400 s timed out both >400 s timed out An unbounded run passed 30 minutes on a 6-second test. Against the 1-hour test timeout that is enough to turn a fault into a timeout, hiding the thing the diagnostics exist to explain. With them removed the same test runs in 5.57 s under the harness's own defaults. The claim that they were "inert until the runtime is already aborting" was an inference from their documentation, never a measurement, and the Frontier A/B that seemed to confirm it ran on CCE -- whose offload runtime ignores libomptarget variables entirely. That measured a lane where they do nothing and was read as evidence they cost nothing anywhere. The ROCm debug agent stays: interleaved on the same machine it costs ~3-4% (5.52/5.34 vs 5.28/5.17, n=2) and it names the faulting kernel and source line, which subsumes the one line of allocation verdict that was lost. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
902 added lines against this repo's ~100-line guidance, which I never stopped to justify -- it grew a fix at a time. Now 649. Tests: 545 lines and 34 cases down to 310 and 16. The overlap was real (three separate tests of the same fault-signature matcher, two of the same symbol forms, format tests duplicated by a combined one) and several docstrings retold this session's history at length rather than saying why the assertion exists. Every one of the five bugs this file was written to catch is still caught -- re-verified by reintroducing each: the bracket marker, the ROCm 6.3.1-only regex, an expensive OFFLOAD_TRACK_* variable, the debugger hijack, and bench.py tailing instead of summarizing. Comments: gpu_diagnostics.py 218 to 200, with the measured tables kept and the post-mortems dropped. The numbers are the part a future reader needs; the story of how I got them is not. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
sbryngelson
changed the title
sbryngelson
changed the title
Restores the deliberate out-of-bounds device write at m_time_steppers.fpp:486, so a real GPU fault appears in an actual job log on all four lanes. That is the only way to judge whether these diagnostics are worth having: a bare address before, and the agent naming s_tvd_rk$m_time_steppers_$ck_L486_16 -- collapsed from ~14k lines to ~35 -- after. Applied as a patch onto current HEAD rather than by taking the file from c2d0579: master was merged into this branch since, and that file changed (#1762), so restoring the old copy would have silently reverted someone else's work. Ground truth is m_time_steppers.fpp:486, s_tvd_rk. Anything naming s_write_run_time_information is the async-attribution failure, not a finding. Revert before merging. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
The section separators added while trimming the test file (# --- detection ---) hit lint_source.py's junk-separator rule and failed CI's Lint Toolchain gate. I did not catch it because I had stopped running precheck: it fails on this machine for two unrelated environmental reasons (a corrupted h5py in the shared venv, and example-case cache clobbering), so I had been committing with --no-verify and reading the failures as noise. A real, catchable violation then hid in that noise. Source lint now passes locally, and the example-case failure has cleared on its own; only the h5py import remains, which CI's clean venv does not have. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
Five fixes, each from something this branch's CI actually did.
1. The preflight excluded three healthy Phoenix nodes and failed the run two jobs deep. Open MPI's default binding kills the process before the binary launches ("hwloc_set_cpubind returned Error for bitmap 0"), and that launch failure was read as "this node cannot run MFC". The probe now passes --bind-to none (a single-rank probe has nothing to bind), treats a launch that never happened as inconclusive rather than a verdict, and falls back to a bare launcher if these options are rejected -- otherwise a launcher that refuses a flag would silently switch the preflight off instead of failing loudly.
2. Every SLURM job's output was printed two to three times: the monitor streams with tail -f and then cats the whole file again. Measured at 3 copies of each line, and 65,000 lines of offload diagnostics repeated for one fault. The reprint is now bounded when streaming succeeded and complete only when it did not, which is the case it existed for.
3. Benchmark jobs are prefixed "Bench | ...". They shared check names with Test Suite jobs on the same cluster and device, and a benchmark failure repeatedly read as a test-suite regression.
4. The run summary page now carries the verdict: an infrastructure fault names the node and says it is not a code failure, and a GPU memory fault shows the faulting kernel. Previously either meant opening a log of tens of thousands of lines.
5. The missing-agent-report warning now names truncation as a cause; on this branch's CI it fired 12-39 times per shard, all from runs killed mid-dump rather than any format change.
Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
A transient PyPI/uv fetch failure was recorded as a cluster-wide outage, which then skipped every other job on that cluster for 20 minutes. On this branch's CI that turned one bad download into red AMD lanes whose tests had already reported 0 failed. The breaker was justified by 17 Frontier jobs spending ~33 minutes each rediscovering an outage -- but the dependency install happens before any compute is committed, on the login node for Frontier, so a failed fetch costs no allocation and there is nothing to protect the matrix from. uv already retries internally. Removes classify-build-failure.sh entirely: this was its only rule. Net -128 lines. The outage breaker itself stays for cases that genuinely are cluster-wide; nothing marks one for a download any more. Claude-Session: https://claude.ai/code/session_013573Qr8zEMdYLkP4XyVfiy
Both shipped with only a bash -n syntax check behind them. Running them for real: an infrastructure fault names the node and says it is not a test failure, a GPU fault puts the faulting kernel on the summary page, a successful job writes nothing, and the job's output is no longer reprinted in full. The de-duplication test asserts volume rather than a marker line: tail -f on an already-complete file shows only its last lines, while in CI it follows the file as it is written, so a marker-based test measures the harness rather than the behaviour. All four are verified red against the previous code.
Nothing marked an outage any more once the PyPI classifier went, so the two remaining check call sites could only ever act on a stale marker -- which is exactly what happened: a marker written before that fix landed kept failing the CCE cpu lane afterwards. The design decision behind removing it rather than repairing it: the breaker converts a local failure into a global one, and every recorded instance of it firing was a false positive caused by something else (a syscheck install timeout, a compile error, and #1813's retry deleting build/venv so attempt 2 could not reach PyPI). It reddened #1805, #1807 and #1811 for twenty minutes at a time. Meanwhile the cost it was protecting against is small: Frontier fetches dependencies on the login node, before any allocation is committed. Removes ci-outage.sh, both check call sites, the exit-78 relay through monitor_slurm_job.sh and run_monitored_slurm_job.sh, and the tests that pinned all of it. The node-fault path (77) is untouched and still covered -- 28 tests across preflight, monitor and requeue still pass. Net -319 lines.
The cluster-wide breaker was removed in be95dbb, so a failed reinstall is no longer misread as an outage. The reason to keep build/venv stands on its own: a compute node has no route to PyPI, so removing it made every retry fail on a fetch that could not succeed. Claude-Session: https://claude.ai/code/session_01Durz16Zuhpse4qPcKjYesB
The injection has done its job. In production CI it produced, on every lane: CCE OpenACC s_tvd_rk$m_time_steppers_$ck_L486_6 and OpenMP _ck_L486_16 via the agent summary (14,276 lines collapsed to ~35), AFAR __omp_offloading_..._QMm_time_steppersPs_tvd_rk_l486, and NVHPC "Function: s_tvd_rk:438, Line: 486" unaided. Taken from current master rather than an old commit, so nothing else on that file is reverted. The PR is now Python and shell only, with no Fortran change at all.
sbryngelson
changed the title
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.
Why this matters now
Across the 8 most recent failed Test Suite runs on other branches, 18 of 27 failing jobs — two thirds — are infrastructure, and both dominant causes are bugs this PR fixes:
Both were self-inflicted by CI machinery added in #1797. They are currently degrading
flame-mixlyr-3D,debug-wall-collisions,cont_damage_fixand others.What it fixes
The preflight was excluding healthy nodes. Open MPI's default binding kills the process before the binary launches (
hwloc_set_cpubind returned "Error"), and that launch failure was read as "this node cannot run MFC" — three healthy Phoenix nodes excluded, two jobs deep, then the run gave up. The probe now passes--bind-to none, treats a launch that never happened as inconclusive rather than a verdict, and falls back to a bare launcher if those options are rejected.A flaky download reddened whole clusters. A transient PyPI failure was recorded as a cluster-wide outage that skipped every other job on that cluster for 20 minutes — including jobs whose tests had already passed. Root cause was
retry_builddeletingbuild/venv(#1813), so attempt 2 could not reinstall from a compute node with no route to PyPI. Both the cause and the breaker are fixed; the dependency install happens on the login node, before any allocation is committed, so there is nothing to protect the matrix from.Logs were printed 2–3 times. The monitor streamed with
tail -fand thencat'd the whole file again — measured at three copies of every line, and 65,000 lines of offload diagnostics repeated for one fault.Benchmark jobs shared check names with Test Suite jobs on the same cluster and device, so a benchmark failure read as a test-suite regression. Now prefixed
Bench | ….The run summary page now carries the verdict — an infrastructure fault names the node and says it is not a code failure; a GPU fault shows the faulting kernel. Previously either meant opening a log of tens of thousands of lines.
GPU fault diagnostics
A memory fault used to reach CI as a bare address. It now names the kernel and source line, verified in production CI on every lane against a deliberately injected out-of-bounds write at
m_time_steppers.fpp:486(since reverted — this PR contains no Fortran change):s_tvd_rk$m_time_steppers_$ck_L486_6s_tvd_rk$m_time_steppers_$ck_L486_16__omp_offloading_..._QMm_time_steppersPs_tvd_rk_l486Function: s_tvd_rk:438,Line: 486(unaided)The ROCm debug agent supplies this on the CCE lanes, where no
CRAY_ACC_*variable can — its trace named the wrong kernel in 81 of 102 traced faults, because dispatch is asynchronous.summarize_rocm_debug_agentcollapses ~14,000 lines to ~35. NVHPC's own wording was also added to the fault signatures: 189 faults on a Phoenix shard were previously not recognised as GPU faults at all.Cost, measured over four interleaved pairs on Frontier CCE: no effect detected on a healthy run (resolution ~0.8%), no log output until something faults, and +0.387 s on a faulting run — 0.011% of the test timeout, so a fault cannot become a timeout.
Deliberately not included
CRAY_ACC_DEBUG— names the routine owning the sync point, not the bad write. A confident wrong suspect is worse than silence.OFFLOAD_TRACK_ALLOCATION_TRACES/_NUM_KERNEL_LAUNCH_TRACES— shipped, then removed: they instrument every allocation and kernel launch, and on an MI210 either alone turned a 5.94 s test into a >400 s timeout.-hacc_model=auto_async_none— would fix CCE attribution, but it is a compile flag and would stop CI exercising the async dispatch production uses.An explicit
HSA_TOOLS_LIBorHSA_ENABLE_DEBUGfrom the caller is respected:mfc.sh testandmfc.sh benchare developer commands, and the agent is mutually exclusive with a GPU core dump.Fixes #1813. Refs #1801, which records the full four-lane measurements and the two items that outlive this PR: the summarizer's format fragility across ROCm versions, and whether the agent should run on lanes that already self-attribute. Supersedes #1814.