Skip to content

Blame the right rank, rather than require every rank to have dumped - #634

Merged
lmoresi merged 2 commits into
developmentfrom
bugfix/hang-report-test-robustness
Aug 23, 2026
Merged

Blame the right rank, rather than require every rank to have dumped#634
lmoresi merged 2 commits into
developmentfrom
bugfix/hang-report-test-robustness

Conversation

@lmoresi

@lmoresi lmoresi commented Aug 23, 2026

Copy link
Copy Markdown
Member

test_0054's end-to-end check asserted the waiting group was exactly [0, 2, 3]. On an oversubscribed CI runner a rank can be scheduled too little to dump inside the window, and the group came back as [0] — a true report of a slower machine, not a defect. It went red on development after #630 merged.

What the tool has to get right is which rank is blamed, so that is what is asserted now: the majority group sits at the collective, rank 1 is not in it, the group contains only ranks that entered it, and rank 1 is named as the odd one out. Requiring a particular number of witnesses tested the scheduler.

7 passed locally.

Underworld development team with AI support from Claude Code

test_0054's end-to-end check asserted the waiting group was exactly [0, 2, 3].
On an oversubscribed CI runner a rank can be scheduled too little to dump inside
the window, and the group came back as [0] -- a true report of a slower machine,
not a defect. It went red on development after #630 merged.

What the tool has to get right is which rank is BLAMED, so that is what is
asserted now: the majority group sits at the collective, rank 1 is not in it,
the group contains only ranks that entered it, and rank 1 is named as the odd
one out. Requiring a particular number of witnesses tested the scheduler.

Underworld development team with AI support from Claude Code
Copilot AI lite review requested due to automatic review settings August 23, 2026 07:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

At a 1.0 s watchdog and a 25 s window the end-to-end check passed here and
failed on CI with the majority located in `importlib._bootstrap`: four
oversubscribed ranks take longer to `import underworld3` than the watchdog
allowed, so the dump file filled with import frames and the job was killed
before the collective produced enough dumps to outvote them. The timings were
tuned to a workstation.

5 s watchdog, 75 s window. The assertion also now names the frame the ranks were
actually found in -- "not at the collective" without saying where cost a CI
round trip to diagnose.

Underworld development team with AI support from Claude Code
@lmoresi
lmoresi merged commit 556b8cc into development Aug 23, 2026
2 checks passed
@lmoresi
lmoresi deleted the bugfix/hang-report-test-robustness branch August 23, 2026 11:03
lmoresi added a commit that referenced this pull request Aug 23, 2026
#632's CI failure was test_0054 from the merged #630, not anything in this
branch. Merging development brings in #634 so the re-run is clean.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants