Skip to content

fix(ci): 🐛 let the C++ MPI test variants oversubscribe under OpenMPI 5 - #324

Merged
robertodr merged 2 commits into
mainfrom
fix/ci-mpi-oversubscribe
Sep 1, 2026
Merged

robertodr merged 2 commits into
mainfrom
fix/ci-mpi-oversubscribe

Conversation

@diagonal-hamiltonian

Copy link
Copy Markdown
Collaborator

🤖 AI text below 🤖

Summary

The C++ mpi ctest variants spawn their own mpiexec -n <n> from inside the test, so the --map-by :OVERSUBSCRIBE that the neighbouring Python steps pass on the command line never reaches them — only the environment can. The variable both justfile recipes already export, OMPI_MCA_rmaps_base_oversubscribe, is the OpenMPI 4 spelling and OpenMPI 5 ignores it.

Measured against OpenMPI 5.0.8, mpiexec -n 4 --host localhost:2 hostname:

env result
none rc=1 — refuses to place
OMPI_MCA_rmaps_base_oversubscribe=1 rc=1 — still refuses
PRTE_MCA_rmaps_default_mapping_policy=:oversubscribe rc=0, 4 ranks

This is latent on main, where monoprop_MPI_TEST_PROCS is "2" and every runner can place 2 ranks — CI stays green either way. It bites any branch that widens the sweep: at 4 ranks the ubuntu-26.04 and macos-15 lanes fail with prte-rmaps-base:alloc-error / "not enough slots", while ubuntu-26.04-arm places it. Both test.yml and the Collect coverage [mpi:on] job are affected, since the coverage recipe runs the same ctest --label-regex mpi.

Found while rebasing #296, which widens the sweep to "2;4" deliberately — at R=2 the peer plan resolves only one peer, so every for k in [0, f) in the sparse transport stays a single iteration and the multi-peer paths never run.

Changes

  • .github/workflows/test.yml: set PRTE_MCA_rmaps_default_mapping_policy (and keep the OpenMPI 4 name) on the Run C++ unit tests under MPI step, which previously set no environment at all.
  • justfile: add the same export to test-mpi and to code-coverage-collect, both of which run ctest --label-regex mpi after exporting only the OpenMPI 4 name.

Checklist

  • Tests added or updated to cover the changes — n/a; this is CI configuration. Verified by direct measurement against OpenMPI 5.0.8 (table above). Note it cannot be proven by this PR's own CI, because main runs the sweep at 2 ranks where the bug is invisible; perf(mpi): 🧭 GF(2)-linear rank routing — messages per rank flat in R #296 exercises it at 4.
  • Documentation updated (docstrings, docs/, CONTRIBUTING.md) if needed — n/a
  • CHANGELOG / release notes updated if applicable — n/a

AI/LLM disclosure

  • I used the following tool to help write this PR description: Claude Code (claude-opus-5)
  • I used the following tool to generate or modify code: Claude Code (claude-opus-5)

The mpi ctest variants spawn `mpiexec -n <n>` inside the test, so the
`--map-by :OVERSUBSCRIBE` the neighbouring Python steps pass on the command line
cannot reach them; only the environment can. Both justfile recipes already
exported OMPI_MCA_rmaps_base_oversubscribe, which is the OpenMPI 4 spelling and
is ignored by 5: measured against 5.0.8, `mpiexec -n 4 --host localhost:2` still
fails with it set and succeeds with PRTE_MCA_rmaps_default_mapping_policy.

No effect while monoprop_MPI_TEST_PROCS is "2", which every runner can place. It
matters for a branch that widens the sweep: at 4 ranks the ubuntu-x86 and macOS
lanes fail with prte-rmaps-base:alloc-error while arm places it, in both test.yml
and the coverage job.

Assisted-by: ClaudeCode:claude-opus-5
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown

Docs preview: https://pr-324.monoprop-docs.pages.dev

@robertodr robertodr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thanks good catch! Does OpenMPI 4 honor the PRTE_MCA_* version?

@diagonal-hamiltonian

Copy link
Copy Markdown
Collaborator Author

thanks good catch! Does OpenMPI 4 honor the PRTE_MCA_* version?

Don't know sorry

Comment thread .github/workflows/test.yml Outdated
Comment thread justfile Outdated
Co-authored-by: Roberto Di Remigio Eikås <robertodr@users.noreply.github.com>
Signed-off-by: Roberto Di Remigio Eikås <robertodr@users.noreply.github.com>
@robertodr
robertodr enabled auto-merge (squash) September 1, 2026 13:49
@robertodr

Copy link
Copy Markdown
Member

thanks good catch! Does OpenMPI 4 honor the PRTE_MCA_* version?

Don't know sorry

I checked, it does not. There was a breaking change in runtime between the versions

@robertodr
robertodr merged commit 6c9d6c7 into main Sep 1, 2026
27 checks passed
@robertodr
robertodr deleted the fix/ci-mpi-oversubscribe branch September 1, 2026 13:55
@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.70%. Comparing base (a24143f) to head (a2ccda4).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #324   +/-   ##
=======================================
  Coverage   97.70%   97.70%           
=======================================
  Files          14       14           
  Lines         742      742           
  Branches       98       98           
=======================================
  Hits          725      725           
  Misses         12       12           
  Partials        5        5           
Flag Coverage Δ
cpp 97.70% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@sonarqubecloud

sonarqubecloud Bot commented Sep 1, 2026

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants