Wait for the evidence, not for the clock - #636
Merged
Merged
Conversation
The end-to-end hang-report test raced a fixed window against machine speed, and failed on CI twice. Both failures were true reports of a slower runner rather than defects: first the waiting group came back short because a rank had not been scheduled enough to dump, then the file filled with `importlib._bootstrap` frames because four oversubscribed ranks took longer to import than the 1 s watchdog allowed. Each time the fix was a bigger number tuned to a workstation, which is not a machine any CI runner resembles. Timing is now out of the test. The script arms its own watchdog AFTER import -- a known point in the program rather than a moment in the schedule -- and writes a marker. The test waits for those markers, then waits for the blocked ranks to have dumped twice, then kills. Import may take as long as it likes. The blocked ranks are blocked until killed, so the wait always completes; the caps are backstops against a broken run, set far above any plausible duration so that reaching one means something is wrong rather than slow. Two things follow. `sorted(biggest_ranks) == [0, 2, 3]` is a legitimate assertion again, because every blocked rank is now waited for by construction instead of hoped for -- the weaker "which rank is blamed" form was a concession to the race. And the test runs in 8.6 s instead of 86, because it kills as soon as the evidence exists rather than sitting out a window. Arming from the script leaves `UW_HANG_WATCHDOG` uncovered, so a separate single-rank test covers it: that entry point is the documented one, and the reason arming happens at import is that a rank which diverges before reaching a `watch()` call reports nothing. 8 passed. Underworld development team with AI support from Claude Code
`_check_velocity_preconditioner` asked `self.snes` for its fieldsplit sub-KSPs.
On the rotated free-slip path that KSP is never set up -- that path builds its
own -- so PETSc printed a full error banner before the exception could be
caught, making a healthy solve look broken. Skipped by path now.
Suppressing it with `PETSc.Sys.pushErrorHandler('ignore')` was tried first: it
is global state and broke 23 unrelated tests.
29 passed.
Underworld development team with AI support from Claude Code
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.
The end-to-end hang-report test raced a fixed window against machine speed and failed CI twice. Both failures were true reports of a slower runner rather than defects: the waiting group came back short because a rank had not been scheduled enough to dump, then the file filled with
importlib._bootstrapframes because four oversubscribed ranks took longer to import than the 1 s watchdog allowed. Each fix was a bigger number tuned to a workstation, which is not a machine any runner resembles.Timing is now out of it. The script arms its own watchdog after import — a known point in the program rather than a moment in the schedule — and writes a marker. The test waits for those markers, then waits for the blocked ranks to have dumped twice, then kills. Import may take as long as it likes; the blocked ranks are blocked until killed, so the wait always completes. The caps are backstops set far above any plausible duration, so reaching one means something is wrong rather than slow.
Two consequences worth noting:
sorted(biggest_ranks) == [0, 2, 3]is a legitimate assertion again. Every blocked rank is waited for by construction instead of hoped for — the weaker "which rank is blamed" form was a concession to the race.Arming from the script leaves
UW_HANG_WATCHDOGuncovered, so a separate single-rank test covers that path — it is the documented entry point, and the reason arming happens at import is that a rank which diverges before reaching awatch()call reports nothing.8 passed locally.
Underworld development team with AI support from Claude Code