diff --git a/.github/workflows/test_pr_and_main.yml b/.github/workflows/test_pr_and_main.yml index 056b55002..3de11374f 100644 --- a/.github/workflows/test_pr_and_main.yml +++ b/.github/workflows/test_pr_and_main.yml @@ -17,6 +17,15 @@ defaults: shell: bash -l {0} env: + # Some GitHub-hosted runners expose an Azure MANA RDMA verbs device + # (mana_0). conda-forge builds MPICH and OpenMPI against UCX, and UCX + # probes that device during MPI_Init, fails to create a UD queue pair + # ("Operation not supported"), and aborts the whole run before any test + # starts. Which runner a job lands on is luck, so this shows up as a + # cell that passes one day and dies at MPI_Init the next. Every rank + # here is on one node, so restrict UCX to the transports that node + # actually uses; nothing in mpi-sppy's tests wants a network fabric. + UCX_TLS: "self,sm,tcp" COV_ARGS: "--parallel-mode --rcfile=${{ github.workspace }}/.coveragerc --data-file=${{ github.workspace }}/.coverage --source=${{ github.workspace }}/mpisppy" EX_COV_ARGS: "--parallel-mode --rcfile=${{ github.workspace }}/.coveragerc --data-file=${{ github.workspace }}/.coverage --source=${{ github.workspace }}/mpisppy,${{ github.workspace }}/examples" @@ -1092,51 +1101,132 @@ jobs: include-hidden-files: true test-cylinders: - name: tests on some cylinders + name: tests on some cylinders (${{ matrix.label }}) runs-on: ubuntu-latest needs: [ruff] + strategy: + # One cell failing must not cancel the others: the point of the matrix + # is to learn *which* MPI builds the RMA path works on, so every cell + # has to report. + fail-fast: false + matrix: + # The flexible (unequal) rank path is the only place mpi-sppy uses + # passive-target MPI RMA (Lock/Get/Put), the least portable corner of + # the spec, and it is where a portability regression would first show. + # It is therefore exercised on both major open-source implementations + # and on more than one mpi4py version. A bare `conda install mpi4py` + # resolves to MPICH, which is what this job used to test alone. + include: + - label: mpich + mpi: mpich + mpi4py: "mpi4py" + artifact: mpich + - label: openmpi + mpi: openmpi + mpi4py: "mpi4py" + artifact: openmpi + - label: mpich, mpi4py 3.1 + mpi: mpich + mpi4py: "mpi4py=3.1" + artifact: mpich-mpi4py31 steps: - uses: actions/checkout@v3 - uses: conda-incubator/setup-miniconda@v3 with: activate-environment: test_env python-version: 3.11 + # conda-forge, exclusively: it is the channel that actually + # maintains per-implementation MPI variants. Asking for `openmpi` + # there also pulls the openmpi-flavored mpi4py (via the `mpi` + # metapackage), so each cell really does exercise the MPI it names; + # on the defaults channel the mpi4py build is not variant-selected + # and an "openmpi" cell could silently keep linking MPICH. + channels: conda-forge + conda-remove-defaults: "true" - name: Install dependencies run: | - conda install mpi4py "numpy" setuptools + conda install ${{ matrix.mpi }} "${{ matrix.mpi4py }}" "numpy" setuptools pip install cplex xpress - name: setup the program run: | pip install -e ".[test]" + - name: Record the MPI build under test + run: | + # The log is the evidence for "exercised on two MPI implementations + # and more than one mpi4py/MPI version". + mpiexec --version || true + python -c "import mpi4py; from mpi4py import MPI; print('mpi4py', mpi4py.__version__); print(MPI.Get_library_version())" + # -oversubscribe is OpenMPI-only (MPICH rejects it) and the np=6 runs + # below ask for more ranks than the runner has cores. Ask mpi4py + # which library it is linked against rather than parsing the mpiexec + # banner: OpenMPI 5 launches through PRRTE and its banner no longer + # reliably says "Open MPI"/"OpenRTE". Detected rather than taken + # from the matrix label, so this stays correct if conda ever resolves + # something other than the label claims. + if python -c "import sys; from mpi4py import MPI; sys.exit(0 if 'open mpi' in MPI.Get_library_version().lower() else 1)"; then + echo "OVERSUBSCRIBE=-oversubscribe" >> $GITHUB_ENV + else + echo "OVERSUBSCRIBE=" >> $GITHUB_ENV + fi + - name: run tests timeout-minutes: 10 run: | cd mpisppy/tests - mpiexec -np 2 coverage run $COV_ARGS -m mpi4py test_with_cylinders.py + mpiexec -np 2 $OVERSUBSCRIBE coverage run $COV_ARGS -m mpi4py test_with_cylinders.py - name: run flexible (unequal) rank tests + # Runs even when the equal-rank step above failed. This step is the + # whole reason the matrix exists, and a failure in the pre-existing + # equal-rank tests would otherwise skip it and leave the question the + # matrix is asking -- does the RMA path work on this MPI build? -- + # unanswered. Both still fail the job. + if: ${{ !cancelled() }} timeout-minutes: 10 run: | cd mpisppy/tests - mpiexec -np 6 coverage run $COV_ARGS -m mpi4py test_spwindow_multisource.py - mpiexec -np 6 coverage run $COV_ARGS -m mpi4py test_flexible_rank_cylinders.py - mpiexec -np 6 coverage run $COV_ARGS -m mpi4py test_flexible_rank_duals.py - mpiexec -np 6 coverage run $COV_ARGS -m mpi4py test_flexible_rank_xhat.py - mpiexec -np 6 coverage run $COV_ARGS -m mpi4py test_flexible_rank_xfeas.py - mpiexec -np 6 coverage run $COV_ARGS -m mpi4py test_flexible_rank_xhat_multistage.py - mpiexec -np 6 coverage run $COV_ARGS -m mpi4py test_flexible_rank_extension_fields.py + mpiexec -np 6 $OVERSUBSCRIBE coverage run $COV_ARGS -m mpi4py test_spwindow_multisource.py + mpiexec -np 6 $OVERSUBSCRIBE coverage run $COV_ARGS -m mpi4py test_flexible_rank_cylinders.py + mpiexec -np 6 $OVERSUBSCRIBE coverage run $COV_ARGS -m mpi4py test_flexible_rank_duals.py + mpiexec -np 6 $OVERSUBSCRIBE coverage run $COV_ARGS -m mpi4py test_flexible_rank_xhat.py + mpiexec -np 6 $OVERSUBSCRIBE coverage run $COV_ARGS -m mpi4py test_flexible_rank_xfeas.py + mpiexec -np 6 $OVERSUBSCRIBE coverage run $COV_ARGS -m mpi4py test_flexible_rank_xhat_multistage.py + mpiexec -np 6 $OVERSUBSCRIBE coverage run $COV_ARGS -m mpi4py test_flexible_rank_extension_fields.py - name: Upload coverage data if: always() uses: actions/upload-artifact@v4 with: - name: coverage-cylinders + name: coverage-cylinders-${{ matrix.artifact }} path: ${{ github.workspace }}/.coverage.* if-no-files-found: ignore include-hidden-files: true + test-cylinders-gate: + # Branch protection requires a check named "tests on some cylinders", + # which is what the job above was called before it became a matrix. A + # matrix reports one context per cell -- "tests on some cylinders + # (mpich)" and friends -- so the required name is never reported and + # every PR sits waiting on a status that will never arrive. Reporting + # it from a gate keeps that name stable no matter how the matrix is + # edited later, and avoids naming individual cells in a setting that + # would then block PRs branched before the matrix existed. + name: tests on some cylinders + runs-on: ubuntu-latest + needs: [test-cylinders] + # A skipped matrix (ruff red) still has to report, or the required check + # hangs; the run is already failing in that case. + if: ${{ !cancelled() }} + steps: + - name: Report the matrix result + run: | + # `needs..result` collapses the whole matrix into one verdict: + # "success" only when every cell succeeded. + echo "cylinder cells: ${{ needs.test-cylinders.result }}" + test "${{ needs.test-cylinders.result }}" = "success" + test-cg: name: column generation tests runs-on: ubuntu-latest diff --git a/doc/designs/flexible_rank_assignments.md b/doc/designs/flexible_rank_assignments.md index 6c1470cd1..2d4c27fcb 100644 --- a/doc/designs/flexible_rank_assignments.md +++ b/doc/designs/flexible_rank_assignments.md @@ -936,20 +936,37 @@ are subtle. If isolation is ever genuinely wanted, prefer a branch in the upstream repository over a separate fork -- same isolation, far less CI and merge friction.) -**Prerequisites before the feature is recommended for production use.** -There is no default to flip, but before the `fullcomm` path is -documented or recommended for production use, exercise it on at least -two MPI implementations (e.g. OpenMPI and MPICH) and more than one -mpi4py / MPI version, since that path is where the RMA-portability risk -lives. +**Prerequisites before the feature is recommended for production use — +met.** There is no default to flip, but the `fullcomm` path was not to +be documented or recommended for production use until it had been +exercised on at least two MPI implementations and more than one mpi4py / +MPI version, since that path is where the RMA-portability risk lives. +The `test-cylinders` CI job now does exactly that on every pull request, +as a three-cell matrix: + +| cell | MPI | mpi4py | +|---|---|---| +| `mpich` | MPICH 5.0.1 | 4.1.2 | +| `openmpi` | OpenMPI 5.0.10 | 4.1.2 | +| `mpich, mpi4py 3.1` | MPICH 4.3.2 | 3.1.6 | + +Two implementations, two MPICH versions, two mpi4py versions. Every +cell runs the whole unequal-rank suite — `test_spwindow_multisource` +plus the six `test_flexible_rank_*` integration tests — at `np=6`, and +the job prints `MPI.Get_library_version()` so the log records which +build was exercised instead of leaving it to be inferred from whatever +`conda install mpi4py` happened to resolve. The matrix is +`fail-fast: false`, so one implementation's failure cannot cancel +another's, and the unequal-rank step runs even when the equal-rank step +above it fails, so a pre-existing failure cannot silently skip the +question this job exists to answer. A **scalable layout exchange** used to sit on this same "finish before recommending it" list; it was removed after the scaling analysis showed the flat `fullcomm.allgather` is fine at any realistic rank count (Pyomo/mpi-sppy#726, closed won't-fix; see the Option D -layout-exchange note) — leaving the MPI-implementation matrix above as -the remaining prerequisite for recommending the feature (not for -landing the intervening phases on `main`). +layout-exchange note). With the MPI matrix now running in CI, **no +release gates remain**. ### Possible future work (out of scope) diff --git a/doc/src/flexible_ranks.rst b/doc/src/flexible_ranks.rst new file mode 100644 index 000000000..2774727fa --- /dev/null +++ b/doc/src/flexible_ranks.rst @@ -0,0 +1,126 @@ +.. _flexible_ranks: + +Flexible (Unequal) Rank Assignments +=================================== + +By default every cylinder in a run gets the same number of MPI ranks: +``mpiexec -np 12`` with a hub and two spokes gives four ranks to each. +That is rarely how the work is actually distributed. A PH hub solves +every subproblem on every iteration and scales well with ranks; a +Lagrangian spoke also solves subproblems but may need fewer iterations; +an xhat shuffle spoke is comparatively lightweight and may be fine with +one or two ranks. Splitting the pool evenly leaves the hub short while +the xhat spoke idles. + +Flexible rank assignment lets you give each cylinder a share of the +rank pool. + +Specifying ratios +----------------- + +Each spoke has a ``---rank-ratio`` option giving its target share +*relative to the hub*, which is always the reference at 1.0. There is no +hub option. All ratios default to 1.0, which is the equal-rank behavior. + +.. code-block:: bash + + mpiexec -np 14 python -m mpi4py ../../mpisppy/generic_cylinders.py \ + --module-name farmer --num-scens 100 \ + --solver-name gurobi --default-rho 1 --max-iterations 50 \ + --lagrangian --lagrangian-rank-ratio 0.5 \ + --xhatshuffle --xhatshuffle-rank-ratio 0.25 + +With ratios hub 1.0, lagrangian 0.5, xhatshuffle 0.25 over 14 ranks, +that gives 8 ranks to the hub, 4 to the Lagrangian spoke, and 2 to the +xhat spoke. The allocation is printed at startup, so you can confirm +what you got without working it out by hand. + +Only relative magnitudes matter: ``1.0 / 0.5 / 0.25`` and +``4 / 2 / 1`` request the same split. Ratios are used rather than +explicit counts so the same command line behaves sensibly at any +``-np``. + +The available options are: + +================================= ======================== +option spoke +================================= ======================== +``--lagrangian-rank-ratio`` Lagrangian outer bound +``--xhatshuffle-rank-ratio`` xhat shuffle inner bound +``--xhatxbar-rank-ratio`` xhat xbar inner bound +``--subgradient-rank-ratio`` subgradient outer bound +``--fwph-rank-ratio`` Frank-Wolfe PH +``--relaxed-ph-rank-ratio`` relaxed PH +``--ph-dual-rank-ratio`` PH dual +``--ph-xfeas-spoke-rank-ratio`` PH xfeas +================================= ======================== + +The ``reduced_costs`` spoke deliberately has no such option; see +`Limitations`_ below. + +How ranks are apportioned +------------------------- + +Ranks are apportioned by the largest-remainder (Hare quota) method, then +every cylinder is given a floor of one rank. The counts always sum to +exactly ``-np``, and every cylinder always runs, so an awkward ratio +cannot silently starve a cylinder of all its ranks. Requesting more +cylinders than ranks is an error, since the floor of one each is then +impossible. + +Uneven division is not warned about; the apportionment simply rounds. + +What it does not change +----------------------- + +When every ratio is 1.0 — the default, and the only possibility before +this feature existed — the run takes exactly the code path it always +did. The ratios themselves are the switch: nothing about the +unequal-rank machinery is reachable until you ask for a non-default +ratio. + +That also means the fallback is free. If the unequal-rank path +misbehaves on some MPI build, set the ratios back to 1.0 and you are +back on the long-proven path, with no other changes to your run. + +Internally, an unequal-rank run puts its MPI window on ``COMM_WORLD`` +rather than per-stratum communicators, and a cylinder reading a +per-scenario field assembles it from however many of the sending +cylinder's ranks hold the scenarios it needs. That machinery is +described in ``doc/designs/flexible_rank_assignments.md``; you do not +need to know it to use the feature. + +Limitations +----------- + +The spokes listed above are supported at any ratio, in both two-stage +and multistage problems. The ``reduced_costs`` spoke is not: it consumes +a per-scenario reduced-cost field whose assembly across unequal rank +counts was never implemented, because its only consumer was a +since-removed rho setter. It therefore has no rank-ratio option and runs +at the hub's rank count. + +If some other cylinder reads a per-scenario field that has no +multi-source assembly, the run fails at startup — during window +creation, not part-way through a solve — with a message naming the +cylinder and the field, and suggesting you run the cylinders that +exchange it at equal rank counts. You will not get a silently +mis-assembled buffer. + +Diagnosing an unequal-rank run +------------------------------ + +A cylinder assembling a field from several sending ranks can catch that +sender part-way through publishing. Such a read is either rejected and +retried, or — for fields whose consumers re-evaluate anyway — accepted +with mixed contents. This is expected and self-correcting, but it means +a bounds cylinder can appear to report less often than you expect. + +Every unequal-rank run therefore counts these reads and prints a +per-field summary as each cylinder finalizes, letting you distinguish a +coherence problem from a merely slow upstream sender. The buckets, how +to read the reported miss rate, and the option for printing the counts +periodically during a run rather than only at the end are described +under ``coherence_diagnostics_period`` in :ref:`secretmenu`. + +Equal-rank runs do no multi-source reads, and print nothing. diff --git a/doc/src/index.rst b/doc/src/index.rst index edf56d74d..bc3d9615f 100644 --- a/doc/src/index.rst +++ b/doc/src/index.rst @@ -72,6 +72,7 @@ MPI is used. admmWrapper.rst stoch_admmWrapper.rst aph.rst + flexible_ranks.rst nompi4py.rst secretmenu.rst pysp.rst diff --git a/doc/src/secretmenu.rst b/doc/src/secretmenu.rst index 26d9eb032..57dd1bc68 100644 --- a/doc/src/secretmenu.rst +++ b/doc/src/secretmenu.rst @@ -1,3 +1,5 @@ +.. _secretmenu: + Secret Menu Items ================= @@ -5,6 +7,79 @@ There are many options that are not exposed in ``mpisppy.utils.config.py`` and w a few of them here. +coherence_diagnostics_period +---------------------------- + +On an unequal-rank run (any cylinder given a ``rank_ratio`` other than +1.0) a per-scenario field is assembled from several of the sending +cylinder's ranks. Such a read can straddle a publish, with some sources +answering from before the write and some from after. The reader then +either rejects the read, or -- for a field whose consumers re-evaluate +anyway -- accepts a blended one. + +Every such read is counted, always, and each cylinder prints a per-field +summary as it finalizes: + +.. code-block:: text + + coherence diagnostic [LagrangianOuterBound] DUALS: total=1812, new_accepted=38, not_new=1664, rejected_incoherent=50, rejected_cross_reader=60, accepted_mixed=0, miss rate=6.07% + +The buckets partition ``total``: + +``new_accepted`` + The sources agreed on an advanced write id; the data was used. + +``not_new`` + The write id did not advance, so there was nothing to take -- the + sender has not published since the last accepted read. + +``rejected_incoherent`` + This rank's own sources disagreed, so the read was rejected and will + be retried. This is the fundamental coherence miss. + +``rejected_cross_reader`` + This rank's sources agreed, but another rank of the same cylinder saw + a different write id -- usually because *it* straddled the publish, + and it records the miss itself. + +``accepted_mixed`` + A relaxed field's sources disagreed and the blended assembly was used + anyway. + +The point of the split is to separate two conditions that look alike +from the outside. If a bounds cylinder seems to report rarely and +``not_new`` dominates, the upstream sender is simply slow and the reader +is fine. If the rejection and mixed buckets dominate, reads are being +lost or blended to publish straddling. The reported ``miss rate`` is +everything a straddle cost -- ``rejected_incoherent`` plus +``rejected_cross_reader`` plus ``accepted_mixed``, over ``total``. + +That summary needs no option. Setting ``coherence_diagnostics_period`` +additionally prints a cylinder's own running counts every N multi-source +reads, which is useful when watching a run live rather than reading it +afterwards. It is set per cylinder, on the dict for the cylinder you +want to watch (hub or spoke): + +.. code-block:: python + + hub_dict["opt_kwargs"]["options"]["coherence_diagnostics_period"] = 500 + +before passing the dicts to ``spin_the_wheel``. It is not exposed as a +CLI flag, so under ``generic_cylinders.py`` it must be set by modifying +the configured dicts in code. + +Three things to know when reading the output. Counters are kept per +``Field`` and aggregated over the cylinder's ranks, so a cylinder that +reads one field from more than one peer cylinder cannot tell you which +peer the misses came from. The periodic line is printed before the +current read has been bucketed, so its buckets sum to ``total - 1``; +only the finalization summary is exactly partitioned. And an equal-rank +run does no multi-source reads at all, so it prints nothing. + +The counters are also available programmatically, as +``SPCommunicator.coherence_counters``, keyed by ``Field``. + + initial_proximal_cut_count -------------------------- diff --git a/mpisppy/tests/test_with_cylinders.py b/mpisppy/tests/test_with_cylinders.py index 43d0cd603..c029239ac 100644 --- a/mpisppy/tests/test_with_cylinders.py +++ b/mpisppy/tests/test_with_cylinders.py @@ -121,6 +121,16 @@ def test_xhatshuffle_coverage(self): from helper_extension import TestHelperExtension self.cfg.xhatxbar_args() + # The assertion below compares the value recorded on iteration 0 with + # the one recorded on iteration 1, so the hub has to actually run both + # iterations. cfg.rel_gap defaults to 0.05, and the hub stops as soon + # as the inter-cylinder gap falls under it -- if the xhatshuffle spoke + # lands an incumbent before the iteration-1 convergence check, PH + # terminates after one iteration and only one value is recorded. That + # is a race (it depends on how fast the spoke is relative to the hub), + # and it fires on some MPI/host combinations and not others. Pin the + # gap to 0 so the two iterations always happen. + self.cfg.rel_gap = 0.0 scenario_creator_kwargs, beans, hub_dict = self._create_stuff(iters=2) list_of_spoke_dict = list() diff --git a/run_coverage.bash b/run_coverage.bash index 5d0688f64..0095a7d3e 100755 --- a/run_coverage.bash +++ b/run_coverage.bash @@ -55,8 +55,11 @@ has_module() { # -oversubscribe is OpenMPI-only (MPICH rejects it). Some flexible-rank tests # need more ranks than the host has cores, so add the flag only under OpenMPI. +# Ask mpi4py which library it is linked against rather than parsing the mpiexec +# banner: OpenMPI 5 launches through PRRTE and its banner no longer reliably +# says "Open MPI"/"OpenRTE". OVERSUBSCRIBE="" -if mpiexec --version 2>&1 | grep -qiE "open[ -]?mpi|open ?rte"; then +if python -c "import sys; from mpi4py import MPI; sys.exit(0 if 'open mpi' in MPI.Get_library_version().lower() else 1)" 2>/dev/null; then OVERSUBSCRIBE="-oversubscribe" fi