diff --git a/docs/content/docs/features/parallelism.mdx b/docs/content/docs/features/parallelism.mdx index 836a066ff..0a4d493d4 100644 --- a/docs/content/docs/features/parallelism.mdx +++ b/docs/content/docs/features/parallelism.mdx @@ -131,13 +131,21 @@ sim = MajoranaPropagator(..., comm=MPI.COMM_SELF) ### Multi-node (`MPI.COMM_WORLD`) -Replace the communicator and launch with `mpiexec`: +Pass a duplicate of `MPI.COMM_WORLD` and launch with `mpiexec`: ```python notest from mpi4py import MPI -sim = MajoranaPropagator(..., comm=MPI.COMM_WORLD) +comm = MPI.COMM_WORLD.Dup() +sim = MajoranaPropagator(..., comm=comm) +... +comm.Free() # once sim and its functionals are no longer used ``` +monoprop exchanges point-to-point messages on fixed tags over `comm`, so your own sends and receives +on the same communicator could match them; a receive with `MPI.ANY_TAG` could even take one of +monoprop's messages and leave it waiting. Reserve the duplicate for monoprop, with no point-to-point +traffic of your own on it. It must outlive the propagator and every functional obtained from it. + ```bash mpiexec -n 8 uv run python your_script.py ``` diff --git a/src/monoprop/majorana_propagator.py b/src/monoprop/majorana_propagator.py index 4ad371e53..0072ea386 100644 --- a/src/monoprop/majorana_propagator.py +++ b/src/monoprop/majorana_propagator.py @@ -111,7 +111,10 @@ def __init__( exception applies on top of either. lower_atol: Monomials with ``|coeff| < lower_atol`` are discarded during evolution. upper_atol: Monomials with ``|coeff| > upper_atol`` are kept regardless of complexity. - comm: Optional MPI communicator (must outlive the simulator). + comm: Optional MPI communicator (must outlive the simulator). Pass a duplicate + reserved for monoprop, such as ``MPI.COMM_WORLD.Dup()``: monoprop exchanges + point-to-point messages on fixed tags, which your own sends and receives on the + same communicator, including ``MPI.ANY_TAG`` receives, could match. """ majorana_operator: MajoranaOperator = ( initial_operator diff --git a/src/monoprop/monomial_propagator.py b/src/monoprop/monomial_propagator.py index ca3a711cc..5b840e492 100644 --- a/src/monoprop/monomial_propagator.py +++ b/src/monoprop/monomial_propagator.py @@ -104,7 +104,10 @@ def _init_simulator( lower_atol: Coefficient-truncation threshold, or ``None``. upper_atol: Coefficient-retention threshold, or ``None``. basis_change: Internal per-Majorana basis change (``2 * num_modes`` entries). - comm: Optional MPI communicator (must outlive the propagator). + comm: Optional MPI communicator (must outlive the propagator). Pass a duplicate + reserved for monoprop, such as ``MPI.COMM_WORLD.Dup()``: monoprop exchanges + point-to-point messages on fixed tags, which your own sends and receives on the + same communicator, including ``MPI.ANY_TAG`` receives, could match. basis: Engine basis -- ``"majorana"`` (default) or ``"pauli"``. """ num_modes = majorana_operator.num_modes diff --git a/src/monoprop/pauli_propagator.py b/src/monoprop/pauli_propagator.py index ec8719262..009bf4b33 100644 --- a/src/monoprop/pauli_propagator.py +++ b/src/monoprop/pauli_propagator.py @@ -70,7 +70,10 @@ def __init__( so a value approaching ``num_qubits`` is rejected with a ``RuntimeError`` rather than started. lower_atol: Monomials with ``|coeff| < lower_atol`` are discarded during evolution. upper_atol: Monomials with ``|coeff| > upper_atol`` are kept regardless of weight. - comm: Optional MPI communicator (must outlive the propagator). + comm: Optional MPI communicator (must outlive the propagator). Pass a duplicate + reserved for monoprop, such as ``MPI.COMM_WORLD.Dup()``: monoprop exchanges + point-to-point messages on fixed tags, which your own sends and receives on the + same communicator, including ``MPI.ANY_TAG`` receives, could match. """ # The engine takes the Schrodinger cutoff in slots (two per qubit); this API in qubits. schrodinger_cutoff = ( diff --git a/tests/conftest.py b/tests/conftest.py index c6750a97e..e5c700a37 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -20,6 +20,12 @@ allreduce internally. ``serial_comm`` is always COMM_SELF: use it for rank-local state such as ``evolved_operator``, ``contract_partially``, ``size()``, or ``graph_size()``. +Both hand out session-wide duplicates of those communicators, never the predefined handles +themselves, as the parallelism docs tell users to do: monoprop's point-to-point traffic then +cannot match messages that conftest or mpi4py exchange on the predefined communicators. +Each duplicate is created on first use and freed at session teardown: ``Dup`` is collective, and +doing it per test would add a collective to every one of them for no extra isolation. + Under more than one rank each rank gets its own ``--basetemp``; see ``_rank_basetemp``. """ @@ -27,16 +33,21 @@ import getpass import os +import stat import tempfile from pathlib import Path -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING import pytest -from tests.mpi_support import HAS_MPI_BACKEND, MPI, SERIAL_COMM +from tests.mpi_support import HAS_MPI_BACKEND, MPI from tests.reference_data import load_problem if TYPE_CHECKING: + from collections.abc import Iterator + + from mpi4py.MPI import Intracomm + from tests.reference_data import FermionicProblem @@ -58,6 +69,28 @@ def _rank_basetemp() -> Path | None: return temproot / f"pytest-of-{user}" / f"mpi-rank-{MPI.COMM_WORLD.rank}" +def _make_private_dir(path: Path) -> None: + """Create ``path`` if needed and make sure only the current user can use it. + + The checks pytest applies to its own ``pytest-of-``, which an explicit ``--basetemp`` + skips: on a shared temporary root another user could create the predictable name first. + """ + path.mkdir(mode=0o700, parents=True, exist_ok=True) + # Not following symlinks: a swapped-in link would otherwise pass every check below. + info = path.lstat() + if stat.S_ISLNK(info.st_mode): + raise OSError( + f"The temporary directory {path} is a symbolic link. Fix this and try again." + ) + if hasattr(os, "getuid") and info.st_uid != os.getuid(): + raise OSError( + f"The temporary directory {path} is not owned by the current user. " + "Fix this and try again." + ) + if info.st_mode & 0o077: + path.chmod(0o700) + + @pytest.hookimpl(tryfirst=True) def pytest_cmdline_main(config: pytest.Config) -> None: """Give each MPI rank its own ``--basetemp`` unless one was passed explicitly. @@ -67,7 +100,7 @@ def pytest_cmdline_main(config: pytest.Config) -> None: """ if config.option.basetemp is None and (basetemp := _rank_basetemp()) is not None: # pytest creates only the basetemp itself when one is given, not its parent. - basetemp.parent.mkdir(mode=0o700, parents=True, exist_ok=True) + _make_private_dir(basetemp.parent) config.option.basetemp = basetemp @@ -124,26 +157,53 @@ def pytest_runtest_setup(item: pytest.Item) -> None: pytest.skip(f"Test requires {min_size} MPI processes") +@pytest.fixture(scope="session") +def comm_self() -> Iterator[Intracomm | None]: + """Session-wide duplicate of COMM_SELF; ``None`` when monoprop has no MPI backend.""" + if not HAS_MPI_BACKEND: + yield None + return + duplicate = MPI.COMM_SELF.Dup() + yield duplicate + duplicate.Free() + + +@pytest.fixture(scope="session") +def comm_world() -> Iterator[Intracomm | None]: + """Session-wide duplicate of COMM_WORLD; ``None`` when monoprop has no MPI backend.""" + if not HAS_MPI_BACKEND: + yield None + return + duplicate = MPI.COMM_WORLD.Dup() + yield duplicate + duplicate.Free() + + _COMM_PARAMS = ( [pytest.param(None, id="comm_none")] if not HAS_MPI_BACKEND else [ - pytest.param(MPI.COMM_SELF, id="comm_self"), - pytest.param(MPI.COMM_WORLD, id="comm_world"), + pytest.param("comm_self", id="comm_self"), + pytest.param("comm_world", id="comm_world"), ] ) @pytest.fixture(params=_COMM_PARAMS) -def comm(request: pytest.FixtureRequest) -> Any: # noqa: ANN401 - """Parametrized communicator; ``None`` when mpi4py is unavailable or monoprop has no MPI backend.""" - return request.param +def comm(request: pytest.FixtureRequest) -> Intracomm | None: + """Parametrized communicator; ``None`` when mpi4py is unavailable or monoprop has no MPI backend. + + Resolved through the session-scoped duplicates, so parametrizing does not reorder the run. + """ + if request.param is None: + return None + return request.getfixturevalue(request.param) @pytest.fixture -def serial_comm() -> Any: # noqa: ANN401 +def serial_comm(comm_self: Intracomm | None) -> Intracomm | None: """Single-rank communicator for tests that inspect rank-local operator state.""" - return SERIAL_COMM + return comm_self @pytest.fixture( diff --git a/tests/mpi_support.py b/tests/mpi_support.py index be9e3217a..79a9d3222 100644 --- a/tests/mpi_support.py +++ b/tests/mpi_support.py @@ -16,8 +16,6 @@ from __future__ import annotations -from typing import Any - import monoprop try: @@ -27,6 +25,3 @@ HAS_MPI_BACKEND: bool = MPI is not None and bool(monoprop.MPI_ABIS) """True when mpi4py is importable and this monoprop installation ships an MPI backend.""" - -SERIAL_COMM: Any = MPI.COMM_SELF if HAS_MPI_BACKEND else None -"""Single-rank communicator for rank-local checks; ``None`` selects the serial backend.""" diff --git a/tests/test_evolved_operator_coefficients.py b/tests/test_evolved_operator_coefficients.py index a2419a9cb..a28bf7a05 100644 --- a/tests/test_evolved_operator_coefficients.py +++ b/tests/test_evolved_operator_coefficients.py @@ -33,7 +33,6 @@ ) from monoprop.majorana import Majorana, MajoranaOperator from monoprop.pauli import Pauli, PauliOperator -from tests.mpi_support import SERIAL_COMM from tests.reference_data import load_problem DATA = Path(__file__).parent / "data" @@ -48,7 +47,7 @@ def problem(): @pytest.fixture(scope="module") -def prop_and_params(problem): +def prop_and_params(problem, comm_self): """A built propagator over ``problem`` and its parameters, shared across the module. ``evolved_operator`` and ``evolved_operator_coefficients`` leave simulator state alone, so one @@ -58,7 +57,7 @@ def prop_and_params(problem): problem.operator, problem.monomial_circuit.initial_state, cutoff=2 * problem.n_modes, - comm=SERIAL_COMM, + comm=comm_self, ) prop.build_graph(problem.monomial_circuit.to_circuit()) return prop, problem.monomial_circuit.parameters diff --git a/tests/test_mpi_configuration.py b/tests/test_mpi_configuration.py index 8668b5066..34a6cd886 100644 --- a/tests/test_mpi_configuration.py +++ b/tests/test_mpi_configuration.py @@ -159,3 +159,38 @@ def test_basetemp_parent_is_created(monkeypatch, tmp_path): config.option.basetemp = None conftest.pytest_cmdline_main(config) assert Path(config.option.basetemp).parent.is_dir() + + +def _basetemp_for_rank(monkeypatch, temproot: Path) -> Path: + monkeypatch.setattr(conftest, "MPI", Mock(COMM_WORLD=Mock(size=3, rank=2))) + monkeypatch.setenv("PYTEST_DEBUG_TEMPROOT", str(temproot)) + config = Mock() + config.option.basetemp = None + conftest.pytest_cmdline_main(config) + return Path(config.option.basetemp) + + +def test_basetemp_parent_owned_by_another_user_is_refused(monkeypatch, tmp_path): + monkeypatch.setattr(conftest.os, "getuid", lambda: tmp_path.stat().st_uid + 1) + with pytest.raises(OSError, match="not owned by the current user"): + _basetemp_for_rank(monkeypatch, tmp_path) + + +def test_basetemp_parent_symlink_is_refused(monkeypatch, tmp_path): + temproot = tmp_path / "temproot" + temproot.mkdir() + elsewhere = tmp_path / "elsewhere" + elsewhere.mkdir(mode=0o700) + monkeypatch.setattr(conftest.getpass, "getuser", lambda: "someone") + (temproot / "pytest-of-someone").symlink_to(elsewhere) + with pytest.raises(OSError, match="symbolic link"): + _basetemp_for_rank(monkeypatch, temproot) + + +def test_basetemp_parent_is_made_private(monkeypatch, tmp_path): + monkeypatch.setattr(conftest.getpass, "getuser", lambda: "someone") + parent = tmp_path / "pytest-of-someone" + parent.mkdir(mode=0o755) + parent.chmod(0o755) + assert _basetemp_for_rank(monkeypatch, tmp_path).parent == parent + assert parent.stat().st_mode & 0o777 == 0o700