Skip to content

Finish the mpi-pytest migration: the remaining project and crash tests - #695

Open
pancetta wants to merge 3 commits into
masterfrom
mpi-pytest-projects
Open

Finish the mpi-pytest migration: the remaining project and crash tests#695
pancetta wants to merge 3 commits into
masterfrom
mpi-pytest-projects

Conversation

@pancetta

@pancetta pancetta commented Sep 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Follow-up to #694, now based on master.

#694 moved pySDC/tests to mpi-pytest. This
finishes the job: every remaining subprocess-launched MPI test that is worth porting, plus the one
in pySDC/tests I had wrongly excused.

Ported

test ranks note
DAE/test_MPI_sweepers 2, 3 ran a project module as a script
Resilience/test_vdp 1, 2, 5, 8 same
tests/…/test_crash 3 see below
AllenCahn_Bayreuth/test_simple_forcing 2, 4 see below
GPU/test_configs 24 the project was in no CI job at all
compression/test_proof_of_concept 1, 4 libpressio image gets mpi-pytest

Two of my earlier exclusions were wrong

test_crash does not abort MPI. I claimed it "aborts a rank on purpose, needs isolation by
design". It raises a ConvergenceError which is caught inside the test body, and the NaN is written
on one rank but detected collectively. I inferred that from the file name without reading it.
Verified: 2 passed on 3 ranks, 1 serial.

AllenCahn_Bayreuth::test_main_parallel asserted nothing. I had blocked on its
export PYTHONPATH (redundant under the editable install) and HWLOC_HIDE_ERRORS (noise
suppression). The more interesting part is that its only assertion was commented out and stderr
merely produced a warning, so the test passed unless the subprocess hung. Running the benchmark
in-process means an exception in it now fails the test.

A test nobody was running

GPU was not in the project matrix, and the workflow's only pytest targets are pySDC/tests,
projects/${{ matrix.env }}/tests, projects/Monodomain/tests and projects/compression/tests.
So pySDC/projects/GPU/tests was collected by no job.

Wiring it in exposed that test_run has been broken since #586 gave run_experiment a required
dt — it fails with KeyError: 'dt'. A one-line addition to the test's args dict fixes it.

The project job now also passes not cupy: these runners have no GPU, and the GPU project is the
only one with cupy-marked tests.

Deliberately not ported

  • Monodomain — the child is coverage run -p run_MonodomainODE_cli.py with a CLI generated
    from an options dict, and the test streams its stdout live. Porting means giving the project a
    Python API, which is a refactor of the project rather than a test migration.
  • parallelSDC/test_preconditioner_MPI — an MPI run followed by a second, serial subprocess
    that produces a plot, plus OPENBLAS_NUM_THREADS/MKL_NUM_THREADS pinned in the child
    environment. Both are arguably worth revisiting on their own merits; neither belongs in a batch
    migration.

Both keep their launchers and remain correct as they are.

Verified

Locally, at every declared rank count:

DAE                n=2 49, n=3 1, serial 64
Resilience         n=2 2, n=5 2, n=8 2, serial 92
RayleighBenard     n=2 3, n=3 3, serial 19
test_crash         n=3 2, serial 1
AllenCahn_Bayreuth n=2 1, n=4 1, serial 8
GPU                n=24 1, serial 1
compression        discovers 1 4 (libpressio is not available to me; CI is its first real run)

etc/run_mpi_tests.sh's own tests still pass (8), and black/ruff are clean.

Dead code removed

Resilience/tests/test_fault_injection.py imported subprocess and never used it.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Sep 20, 2026 •
edited
Loading

Copy link
Copy Markdown

Coverage report

This PR does not seem to contain any modification to coverable code.

@pancetta
pancetta force-pushed the mpi-pytest-projects branch 4 times, most recently from d9a1be6 to 5a31915 Compare September 21, 2026 07:04
@pancetta @claude
Port the DAE and Resilience project MPI tests to mpi-pytest
c11a99c 
Both had the same shape as the ones in pySDC/tests: a test whose whole body was an `mpirun` of a
project module, with the real work reachable only through that module's `__main__`. They now call
`check_order`, `mpi_vs_nonMPI`, `check_step_size_limiter` and `check_adaptivity_with_avoid_restarts`
directly on the ranks pytest already has, and DAE's `testVersions` loses its `launch` flag.

`test_adaptivity_with_avoid_restarts` used to invoke the launcher with an argument that matched none
of vdp's dispatch branches and fell through to a condition that is always true
(`elif 'check_adaptivity_with_avoid_restarts' and size == 1`). It now calls the function it meant.

The project job takes a table of project -> rank counts rather than a special case for one project.

Verified locally at every declared rank count:
  DAE            n=1 64 passed, n=2 49, n=3 1        partition 117 = 117
  Resilience     n=1 92 passed, n=2 2, n=5 2, n=8 2  partition  98 =  98
  RayleighBenard n=1 19 passed, n=2 3, n=3 3         partition  25 =  25

DAE's n=1 pass also reports one failure, `test_DiscontinuousTestDAE_SDC_detection[5]`, on a
tolerance. That file is byte-identical to master and fails there too; it is not related to this
change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@pancetta
pancetta changed the base branch from mpi-pytest-migration to master September 21, 2026 10:06
@pancetta @claude
Finish the subprocess removal: crash, AllenCahn_Bayreuth, GPU and com…
c8f174f 
…pression

Reviewing the remaining launchers turned up two of my own earlier judgements as wrong.

`test_crash` does not abort MPI. It raises a `ConvergenceError` that is caught in the test body, and
the NaN is written on one rank but detected collectively -- so it never needed isolation. I had
inferred that from the file name. Ported and verified on 3 ranks.

`AllenCahn_Bayreuth::test_main_parallel` was blocked in my earlier review on its `export PYTHONPATH`
and `HWLOC_HIDE_ERRORS`, which are respectively redundant under the editable install and pure noise
suppression. More to the point it **asserted nothing**: its one assertion was commented out and
stderr merely raised a warning, so it passed unless the subprocess hung. Running the benchmark here
means an exception in it now fails the test.

The GPU project was not in the matrix, so `pySDC/projects/GPU/tests` was collected by no job at all.
Wiring it in exposed that `test_run` has been broken since #586 gave `run_experiment` a required
`dt`: it fails with `KeyError: 'dt'`. One line to the test's args dict fixes it. The project job now
also deselects `cupy`, since these runners have no GPU and the GPU project is the only one with such
tests.

compression is ported too; the libpressio image gets mpi-pytest and the runner.

Verified locally where possible: test_crash 2 passed on 3 ranks and 1 serial; GPU n=24 1 passed plus
1 serial; AllenCahn_Bayreuth n=2 1, n=4 1, 8 serial; compression discovers 1 4 (libpressio itself is
not available here, so CI is its first real run). Runner tests still 8 passed.

Only three launchers remain: Monodomain and parallelSDC, which are argued in the PR description, and
this suite's own stub in test_run_mpi_tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@pancetta pancetta changed the title Port the DAE and Resilience project MPI tests to mpi-pytest Finish the mpi-pytest migration: the remaining project and crash tests Sep 21, 2026
@pancetta @claude
Give the GPU project the CuPy stub, and the dependencies its tests need
cacff2a 
Two things the newly wired GPU job exposed.

Its environment was missing mpi4py-fft, which `spectral_helper` needs -- the job failed with
`ModuleNotFoundError: No module named 'mpi4py_fft'`. It now mirrors the RayleighBenard environment,
since this project's configs import that project's and both go through the same spectral machinery.

And no, the GPU project's `cupy`-marked tests were not using the stub from #692: that is activated by
`pySDC/tests/conftest.py`, which only applies under `pySDC/tests`. They have never run anywhere. A
conftest in the project's test directory reuses the same activation, and the project job gets the
same stub step the mpi4py job has -- outside the coverage run, so it cannot claim GPU code is
covered.

That turned up one stub infidelity: CuPy's iterative solvers still take `tol=`, which SciPy renamed
to `rtol` and then removed, so `HeatEquation_ND_FD_CuPy`'s `cg(..., tol=...)` failed against the
stub. Since the stub is impersonating CuPy it has to accept CuPy's spelling, so `tol` is translated
for cg, gmres, bicgstab, minres, cgs and qmr.

Verified: both GPU project cupy tests pass under the stub where they previously did not run at all,
and the main suite's 107 stubbed cupy tests are unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@pancetta