Strip every launcher's variables before the serial reference child (#675) - #681
Merged
Merged
Conversation
) test_parallel_matches_serial_bit_identical spawns a serial reference subprocess from inside an mpirun and scrubs the launcher's variables so the child does not try to join the parent's job. Its scrub list named the Open MPI family only: OMPI_, PMIX_, PRTE_, PRTERUN_, OPAL_PREFIX. MPICH advertises itself through PMI_*. On the Linux CI runner the child therefore inherited PMI_FD and tried to join a job it was not part of: [cli_0]: PMIU_write error; fd=9 buf=:cmd=init pmi_version=1 system msg for write_line failure : Bad file descriptor Abort: internal_Init_thread ... PMI_Init returned -1 assert 15 == 0 macOS runs Open MPI, where that list IS complete, so this passed on one machine and failed on the other with nothing to do with the operating system. It had never been caught because the file matches no glob in either test script and had never run in CI at all. The complete list already existed thirty lines away, in serial_reference, and this test hand-rolled a copy that drifted. It now imports the shared tuple, which gains PRTE_/PRTERUN_/OPAL_ so the union is strictly wider than either copy was -- swapping to the shared list unchanged would have narrowed Open MPI 5 coverage, since PRRTE is a separate family from OMPI_. test_0873_adapt_collective_stop_mpi was only ever collateral: it passes in isolation, and failed downstream of this abort, which also produced the divergence that hung the batch for 76 minutes (rank 0 in _from_plexh5, rank 1 in barrier). Verified on the Linux/MPICH runner, both files, whole: 8 passed in 3.81s. Open MPI 5.0.10 locally: test_0855 4 passed, and test_1069, which uses serial_reference directly, 7 passed 1 skipped. Underworld development team with AI support from Claude Code Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E87Q7KrpapxeQiLD1RiNXv
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped to test subprocess environment scrubbing and correctly centralizes the launcher-variable prefix list to prevent MPICH/Open MPI divergence.
Pull request overview
This PR fixes a cross-MPI-launcher environment inheritance bug in test_parallel_matches_serial_bit_identical by centralizing the “strip launcher variables” logic, ensuring a serial reference subprocess spawned from within an MPI job does not attempt to join the parent MPI world (notably under MPICH via PMI_*).
Changes:
- Replaces the hand-rolled Open MPI–only env-var scrub list in
test_0855with an import of the shared_MPI_ENV_PREFIXEStuple fromserial_reference.py. - Expands the shared scrub-prefix tuple to include Open MPI 5 runtime families (
PRTE_,PRTERUN_,OPAL_) in addition to existing prefixes (includingPMI_*for MPICH).
File summaries
| File | Description |
|---|---|
| tests/parallel/test_0855_mesh_smoothing_parallel.py | Uses the shared MPI launcher env scrub tuple to reliably spawn a serial reference subprocess under multiple MPI implementations. |
| tests/parallel/serial_reference.py | Broadens and documents the shared MPI launcher env scrub tuple to cover additional Open MPI runtime prefixes. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| import underworld3 as uw | ||
| from underworld3.meshing import smooth_mesh_interior | ||
|
|
||
| from serial_reference import _MPI_ENV_PREFIXES |
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.
Summary
Closes #675.
test_parallel_matches_serial_bit_identicalspawns a serial referencesubprocess from inside an
mpirun, and scrubs the launcher's variables so thechild does not try to join the parent's job. Its scrub list named the Open MPI
family only:
MPICH advertises itself through
PMI_*. On the Linux CI runner the childinherited
PMI_FDand tried to join a job it was not part of:macOS runs Open MPI, where that list is complete. So this passed on one
machine and failed on the other, with nothing to do with the operating system —
and it had never been caught because the file matches no glob in either test
script and had never run in CI at all.
The fix already existed thirty lines away
tests/parallel/serial_reference.py, same directory, carries the complete list.This test hand-rolled a copy and the copy drifted. It now imports the shared
tuple.
The shared tuple gains
PRTE_,PRTERUN_andOPAL_, so the union is strictlywider than either copy was. Swapping to the shared list unchanged would have
narrowed Open MPI 5 coverage — PRRTE is a separate family from
OMPI_, andthe local copy was the only thing naming it.
This is Charter §5 copy-paste-as-reuse, and the divergence between the two
copies is the entire bug.
test_0873 was collateral
test_0873_adapt_collective_stop_mpi::test_premise_the_metric_splits_the_rankspasses in isolation. It failed only downstream of this abort, which also
produced the rank divergence that hung the batch for 76 minutes — rank 0 in
_from_plexh5(), rank 1 inbarrier(). Both files are green together now.Verification
test_0855test_1069(usesserial_referencedirectly)The Linux run was obtained with the supervisor from #678, which turned 76
minutes of silence into a named diagnosis in six.
Not fixed here
The coverage hole that hid this — three parallel files matching no glob in
either script — is #615's subject. With this merged, that PR's blocker is gone.
Underworld development team with AI support from Claude Code
🤖 Generated with Claude Code
https://claude.ai/code/session_01E87Q7KrpapxeQiLD1RiNXv