Skip to content

Run the whole parallel directory, and at four ranks as well as two - #615

Open
lmoresi wants to merge 1 commit into
developmentfrom
bugfix/parallel-test-coverage
Open

Run the whole parallel directory, and at four ranks as well as two#615
lmoresi wants to merge 1 commit into
developmentfrom
bugfix/parallel-test-coverage

Conversation

@lmoresi

@lmoresi lmoresi commented Aug 19, 2026

Copy link
Copy Markdown
Member

Addresses part of #570, and is how #611 was found.

Two scripts, two different holes

script glob files named missed
scripts/test.sh (CI runs this) test_075*py + test_10*py 14 of 32 0005, 0700, 0760–0790, 0855, 0873
scripts/test_levels.sh (./uw test) test_07*py 20 of 32 0005, 0855, 0873, all of test_10*

Three files ran at no rank count in either script: test_0005_xdmf_viz_topology_mpi,
test_0855_mesh_smoothing_parallel, and test_0873_adapt_collective_stop_mpi
the last added two days ago in #596 specifically to guard against a parallel
hang, and never once run in parallel by our own tooling.

Both scripts now name the directory. A glob that names ranges grows holes as
files are added between them; test_075* stops at 0759 and test_10* starts at
1000, so everything added in between fell through. That is the #570 class, and it
is why #611 sat undetected.

Four ranks as well as two

Two ranks is a special case. The defect this suite exists to catch is a
collective entered by some ranks and not others, and with two ranks the
mismatched pair often still meets — the skipped branch does not arise on a
two-way partition, or one rank's internal barrier is satisfied by the other's
next collective. Every instance found recently passed at np=2 and hung at np=4:
the conditional collective in #609, and #611.

test_levels.sh already had a --full-parallel flag for exactly this and was
pointing it at the narrower glob.

Measured

Machine otherwise idle, since contention distorts these badly:

result time
tests/parallel/ at np=2 128 passed, 11 skipped 156 s
tests/parallel/ at np=4, 1 deselected 135 passed, 3 skipped 170 s
file-by-file at np=4 31 of 32 pass
end to end, scripts/test.sh --p 2 135 passed, 3 skipped, 1 deselected, rc=0 198 s for the parallel section

The skip counts differ between rank counts because some tests require four ranks
and skip at two — a second reason to run both, independent of the collective
argument.

Cost to CI: roughly three minutes for the added np=4 pass, against the 120-minute
cap. #573 is open on the total.

The one deselection

parallel/test_0760_swarm_cache_migration.py::test_global_evaluate_after_migration
passes at np=2 and hangs at np=4 on development — that is #611, filed
separately, not introduced here.

Note the node id has no tests/ prefix: tests/pytest.ini puts rootdir at
tests/. A deselect that does not match any test is ignored in silence, so a
mistyped node id leaves the test running and the pass hanging, with nothing said
about it. Confirm 1 deselected in the output when changing it.

Underworld development team with AI support from Claude Code

… two

Two scripts, two different holes. scripts/test.sh (what CI runs) used
tests/parallel/test_075*py and test_10*py; scripts/test_levels.sh (what
./uw test runs) used tests/parallel/test_07*py. Of the 32 collectible files in
that directory, test.sh named 14 and missed test_0005, test_0700,
test_0760..test_0790, test_0855 and test_0873; test_levels.sh missed
test_0005, test_0855, test_0873 and the whole test_10* solver set.

Three files therefore ran at NO rank count in either script: test_0005,
test_0855, and test_0873 — the last added two days ago in #596 to guard
against a parallel hang.

Both now name the directory. A glob that names ranges grows holes as files are
added between them, which is the #570 class and is how #611 survived
unnoticed.

Both scripts also run the set at four ranks. Two ranks is a special case: the
defect this suite exists to catch is a collective entered by some ranks and
not others, and with two the mismatched pair often still meets. Every instance
found recently passed at np=2 and hung at np=4 — the conditional collective in
#609, and #611 itself. test_levels.sh already had --full-parallel for this and
was pointing it at the narrower glob.

Measured on this directory, machine otherwise idle:

    np=2   128 passed, 11 skipped     156 s
    np=4   135 passed,  3 skipped     170 s   (1 deselected)

end to end, scripts/test.sh --p 2: 135 passed, 3 skipped, 1 deselected, 198 s
for the parallel section, rc=0. The skip counts differ because some tests
require four ranks and skip at two, which is a second reason to run both.

The single deselection is #611: test_global_evaluate_after_migration passes at
np=2 and hangs at np=4 on development. Its node id carries no `tests/` prefix
because tests/pytest.ini puts rootdir at `tests/`; a deselect that does not
match is ignored in silence, which cost two wrong diagnoses while measuring
this.

Underworld development team with AI support from Claude Code
Copilot AI lite review requested due to automatic review settings August 19, 2026 08:56

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.

@lmoresi

lmoresi commented Aug 19, 2026

Copy link
Copy Markdown
Member Author

Adversarial review

Reviewed at ad43614. Four findings.

1. The deselection is a silent-failure mechanism guarding a silent failure.
--deselect with a node id that does not match is ignored without complaint —
that is precisely what produced two wrong diagnoses while measuring this PR. So
the guard against #611 is itself the kind of thing that can stop working without
saying so: rename the test, move the file, or change pytest.ini's rootdir and
the deselect quietly stops matching and the np=4 pass hangs again. A
--strict-markers-style equivalent does not exist for deselect. Marking the test
@pytest.mark.skipif on rank count, inside the file, would fail loudly if the
name drifted; it also puts the exclusion where a reader of the test will see it.

2. Turning on 18 previously-unrun files rests on one machine. They pass here
at np=2 and np=4, on macOS/arm64 with Open MPI 5.0.10 and --oversubscribe
above the core count. CI is ubuntu-latest with a different MPI and different core
count, and several of these files have never run in that environment at all. The
first CI run of this PR is the real test, and it may surface failures that are
about the runner rather than about the tests. That is a reason to watch it land,
not a reason to keep the hole.

3. The np=4 pass is hard-coded to 4. if [ "$PARALLEL_RANKS" -ne 4 ] then
mpirun -n 4, so asking for --p 8 gives 8 and 4, while --p 4 gives 4 alone.
That is defensible — 4 is the cheapest rank count that exposes the mismatched-
collective class — but it is a convention baked into two scripts rather than a
variable, and someone wanting 2-and-8 has to edit both.

4. test_levels.sh still has two paths where test.sh has one. Its
--full-parallel flag now duplicates what test.sh does unconditionally, so the
same directory is described in two places with slightly different flags. The
duplication predates this PR and is not made worse by it, but the two scripts
having diverged into different globs is exactly what caused the three-file hole,
and they will diverge again.

Checked. bash -n on both scripts; the full scripts/test.sh --p 2 path end
to end at rc=0; and the deselect verified to actually match this time by
confirming 1 deselected in the output, which is the check that was missing
when it silently did not.

Underworld development team with AI support from Claude Code

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