From ff7745d72e305e87c10331c0f80e5aa42233e83e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Di=20Remigio=20Eik=C3=A5s?= Date: Fri, 23 Oct 2026 10:50:30 +0200 Subject: [PATCH 1/2] =?UTF-8?q?feat(build):=20=E2=9C=A8=20declare=20named?= =?UTF-8?q?=20build=20options=20with=20scikit-build-core=201.1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scikit-build-core 1.1 lets a package declare its own config-settings and match overrides on them. Use that to give the source build a documented interface instead of raw CMake defines: - monoprop.mpi, monoprop.sanitizer, monoprop.cxx-unit-tests, monoprop.arch-flags and monoprop.mpi-test-procs, each settable as `-C monoprop.=...` or through the environment variable named after the CMake option (which takes precedence). Unset options drop their define, so CMake's defaults stay authoritative. - monoprop.mpi provisions mpi4py for both routes; overrides could previously only match environment variables, so the config-settings route had to repeat the mpi4py pin. - The sanitizer option also selects the AsanUbsan/Tsan build type, and MPI builds use their own Release-mpi tree, so switching variants rebuilds incrementally instead of reconfiguring a shared tree. - The variant environment variables join tool.uv.cache-keys, so uv rebuilds on a switch without --reinstall-package/--no-cache. The justfile, CI, Nix pin, presets and docs follow. Assisted-by: Pi:claude-opus-5-5 --- .github/workflows/bench.yml | 2 +- .github/workflows/docpages.yml | 2 +- .github/workflows/qa-analysis.yml | 19 ++-- .vscode/c_cpp_properties.json | 1 + CMakePresets.json | 28 ++++++ README.md | 7 +- cpp/tests/README.md | 15 ++-- docs/content/docs/building.mdx | 138 +++++++++++++++++------------- docs/content/docs/testing.mdx | 25 +++--- flake.nix | 6 +- justfile | 31 ++++--- nix/monoprop.nix | 5 +- pyproject.toml | 65 ++++++++++++-- 13 files changed, 226 insertions(+), 118 deletions(-) diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 042c96d9..c2c42fd4 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -53,7 +53,7 @@ jobs: - name: Install package env: - SKBUILD_CMAKE_DEFINE: monoprop_ENABLE_CXX_UNIT_TESTS=OFF + monoprop_ENABLE_CXX_UNIT_TESTS: "OFF" monoprop_ENABLE_MPI: "ON" run: just build --group bench diff --git a/.github/workflows/docpages.yml b/.github/workflows/docpages.yml index d324110b..5305101f 100644 --- a/.github/workflows/docpages.yml +++ b/.github/workflows/docpages.yml @@ -34,7 +34,7 @@ jobs: runs-on: ubuntu-26.04 env: - SKBUILD_CMAKE_DEFINE: monoprop_ENABLE_CXX_UNIT_TESTS=OFF + monoprop_ENABLE_CXX_UNIT_TESTS: "OFF" steps: - name: Checkout diff --git a/.github/workflows/qa-analysis.yml b/.github/workflows/qa-analysis.yml index c2b258ba..00b11666 100644 --- a/.github/workflows/qa-analysis.yml +++ b/.github/workflows/qa-analysis.yml @@ -134,10 +134,9 @@ jobs: runs-on: ubuntu-26.04 permissions: contents: read - # The build type also selects the tree the test recipes run against. + # The profile also selects the build type, hence the tree the test recipes run against. env: - SKBUILD_CMAKE_BUILD_TYPE: "AsanUbsan" - SKBUILD_CMAKE_DEFINE: "monoprop_SANITIZER=asan-ubsan" + monoprop_SANITIZER: "asan-ubsan" steps: - uses: actions/checkout@v7.0.1 @@ -170,8 +169,7 @@ jobs: permissions: contents: read env: - SKBUILD_CMAKE_BUILD_TYPE: "Tsan" - SKBUILD_CMAKE_DEFINE: "monoprop_SANITIZER=tsan" + monoprop_SANITIZER: "tsan" steps: - uses: actions/checkout@v7.0.1 @@ -207,7 +205,7 @@ jobs: - name: Install package env: - SKBUILD_CMAKE_DEFINE: "monoprop_ENABLE_CXX_UNIT_TESTS=OFF" + monoprop_ENABLE_CXX_UNIT_TESTS: "OFF" monoprop_ENABLE_MPI: "ON" run: just build @@ -222,7 +220,7 @@ jobs: SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} with: args: > - -Dsonar.cfamily.compile-commands=build/editable/Release/compile_commands.json + -Dsonar.cfamily.compile-commands=build/editable/Release-mpi/compile_commands.json clang-tidy: needs: [cpp-changes] @@ -250,7 +248,8 @@ jobs: - name: Build (generates compile_commands.json) env: - SKBUILD_CMAKE_DEFINE: "CMAKE_CXX_COMPILER=clang++;monoprop_ENABLE_CXX_UNIT_TESTS=OFF" + SKBUILD_CMAKE_DEFINE: "CMAKE_CXX_COMPILER=clang++" + monoprop_ENABLE_CXX_UNIT_TESTS: "OFF" monoprop_ENABLE_MPI: "ON" run: just build @@ -273,7 +272,7 @@ jobs: mapfile -t clang_tidy_inputs < <( jq -r --arg source_root "$GITHUB_WORKSPACE/cpp/monoprop/" \ '.[].file | select(startswith($source_root)) | select(endswith(".cpp"))' \ - build/editable/Release/compile_commands.json + build/editable/Release-mpi/compile_commands.json ) else clang_tidy_inputs=("${changed_cpp_files[@]}") @@ -285,7 +284,7 @@ jobs: '$files | map({name: ($workspace + .), lines: [[1, 2147483647]]})') run-clang-tidy \ - -p build/editable/Release \ + -p build/editable/Release-mpi \ -source-filter='^${{ github.workspace }}/cpp/monoprop/.*$' \ -header-filter='^(.*/)?cpp/(include/)?monoprop/.*$' \ -exclude-header-filter='^.*/src/monoprop/bindings/.*$' \ diff --git a/.vscode/c_cpp_properties.json b/.vscode/c_cpp_properties.json index 8fade405..1316ce54 100644 --- a/.vscode/c_cpp_properties.json +++ b/.vscode/c_cpp_properties.json @@ -13,6 +13,7 @@ "intelliSenseMode": "linux-gcc-x64", "compileCommands": [ "${workspaceFolder}/build/editable/Release/compile_commands.json", + "${workspaceFolder}/build/editable/Release-mpi/compile_commands.json", "${workspaceFolder}/build/editable/Debug/compile_commands.json", "${workspaceFolder}/build/release-gcc/compile_commands.json", "${workspaceFolder}/build/coverage-gcc/compile_commands.json", diff --git a/CMakePresets.json b/CMakePresets.json index 93bdf9b9..37c53cbb 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -8,6 +8,13 @@ "binaryDir": "${sourceDir}/build/editable/Release", "generator": "Ninja" }, + { + "name": "skbuild-release-mpi", + "displayName": "scikit-build-core (Release, MPI, adopt existing tree)", + "description": "Adopt the MPI Release build tree generated by 'monoprop_ENABLE_MPI=ON uv sync' / scikit-build-core. Do NOT reconfigure this preset from CMake Tools; regenerate the tree with uv/scikit-build-core, then use this preset only to build and run the C++ CTest suite.", + "binaryDir": "${sourceDir}/build/editable/Release-mpi", + "generator": "Ninja" + }, { "name": "skbuild-debug", "displayName": "scikit-build-core (Debug, adopt existing tree)", @@ -28,6 +35,17 @@ ], "jobs": 4 }, + { + "name": "skbuild-release-mpi", + "displayName": "scikit-build-core (Release, MPI) - Build C++ unit tests", + "description": "Build the C++ unit test executable in the scikit-build-core MPI Release tree. Requires the tree to already exist (run 'monoprop_ENABLE_MPI=ON uv sync --all-extras').", + "configurePreset": "skbuild-release-mpi", + "targets": [ + "monoprop_unit_tests.x", + "monoprop_link_export_probe.x" + ], + "jobs": 4 + }, { "name": "skbuild-debug", "displayName": "scikit-build-core (Debug) - Build C++ unit tests", @@ -51,6 +69,16 @@ }, "inheritConfigureEnvironment": true }, + { + "name": "skbuild-release-mpi", + "displayName": "scikit-build-core (Release, MPI) - Run C++ unit tests", + "description": "Run the C++ CTest suite, including the MPI variants, from the scikit-build-core MPI Release tree.", + "configurePreset": "skbuild-release-mpi", + "output": { + "outputOnFailure": true + }, + "inheritConfigureEnvironment": true + }, { "name": "skbuild-debug", "displayName": "scikit-build-core (Debug) - Run C++ unit tests", diff --git a/README.md b/README.md index 4656ad52..91e2b5c7 100644 --- a/README.md +++ b/README.md @@ -110,7 +110,9 @@ plus the `-mpi` lists), which is what CI and the devcontainer install. `just build [uv sync args…]` performs the build CI performs — the same recipe GitHub Actions calls, so a lane can be reproduced locally. -Full instructions — prerequisites, MPI options, and running the example +Build options (MPI, sanitizers, C++ tests) can be set through environment +variables or config-settings such as `-C monoprop.mpi=ON`. +Full instructions — prerequisites, build options, and running the example executable — are in the [building guide](https://docs.monoprop.algorithmiq.tech/building). In particular, from-source builds require `hwloc` and `pkg-config` so CMake can locate `hwloc`. @@ -129,7 +131,8 @@ for the with/without-MPI details and the rank matrix. CI has explicit MPI-enable lanes on Linux x86-64, Linux ARM64, and macOS; installing the `mpi` extra alone does not enable the C++ MPI build. CI requires a registered MPI CTest variant, and each whole-suite MPI run has a 600-second deadlock timeout. Source builds -select the MPI variant with `monoprop_ENABLE_MPI=ON` in the environment. +select the MPI variant with `monoprop_ENABLE_MPI=ON` in the environment (or +`-C monoprop.mpi=ON`). Standalone MPI-enabled C++ programs must initialize MPI before constructing a propagator and finalize it only after all propagators have been destroyed. The exported CMake target preserves the package's MPI setting independently of any diff --git a/cpp/tests/README.md b/cpp/tests/README.md index c69941de..f33e03b1 100644 --- a/cpp/tests/README.md +++ b/cpp/tests/README.md @@ -31,14 +31,15 @@ ctest --test-dir build/editable/Release ``` For an MPI-enabled tree, rerun `uv sync` with `monoprop_ENABLE_MPI=ON` in the -environment. +environment. MPI builds go to their own tree, `build/editable/Release-mpi`, so +the MPI variants below are registered there. ## Running Tests ```bash -ctest --test-dir build/editable/Release # everything -ctest --test-dir build/editable/Release -L serial # serial variants only -ctest --test-dir build/editable/Release -L mpi # MPI variants -ctest --test-dir build/editable/Release -L mpi-2 # only the 2-rank run +ctest --test-dir build/editable/Release # everything +ctest --test-dir build/editable/Release -L serial # serial variants only +ctest --test-dir build/editable/Release-mpi -L mpi # MPI variants (MPI tree) +ctest --test-dir build/editable/Release-mpi -L mpi-2 # only the 2-rank run ``` Or drive the binary directly: @@ -46,7 +47,7 @@ Or drive the binary directly: ```bash build/editable/Release/bin/monoprop_unit_tests.x --list_content build/editable/Release/bin/monoprop_unit_tests.x --run_test=pauli_algebra_* -mpirun -n 2 build/editable/Release/bin/monoprop_unit_tests.x +mpirun -n 2 build/editable/Release-mpi/bin/monoprop_unit_tests.x ``` Because CTest discovery treats each `--list_content` line as a top-level test @@ -137,7 +138,7 @@ runner indefinitely. For exhaustive rank coverage, run `just test-mpi '1;2;4'` or configure `-Dmonoprop_MPI_TEST_PROCS='1;2;4'`. To run a single case under MPI while debugging, invoke the binary directly: -`mpirun -n 2 build/editable/Release/bin/monoprop_unit_tests.x --run_test=`. +`mpirun -n 2 build/editable/Release-mpi/bin/monoprop_unit_tests.x --run_test=`. ## Adding New Tests diff --git a/docs/content/docs/building.mdx b/docs/content/docs/building.mdx index 6225e225..96aabe1c 100644 --- a/docs/content/docs/building.mdx +++ b/docs/content/docs/building.mdx @@ -14,7 +14,7 @@ mechanism differs by build: | Build | Enable MPI with | | --- | --- | -| Python bindings and C++ tests (scikit-build / `uv` / `pip`) | `monoprop_ENABLE_MPI=ON` in the environment | +| Python bindings and C++ tests (scikit-build / `uv` / `pip`) | `monoprop_ENABLE_MPI=ON` in the environment, or the `monoprop.mpi=ON` config-setting | The prebuilt wheels published to PyPI (`pip install monoprop`) are also built without MPI, so a from-source build is required for multi-rank runs. @@ -42,7 +42,7 @@ Flake outputs support x86_64 Linux, aarch64 Linux, and aarch64 macOS. The | --- | --- | | `nix develop` | a shell with every prerequisite above, plus `uv`, `just` and Node.js | | `nix build .#monoprop` | the Python package, built without MPI | -| `nix build .#monoprop-mpi` | the same package with `monoprop_ENABLE_MPI=ON` | +| `nix build .#monoprop-mpi` | the same package with MPI enabled | | `nix build .#monoprop-py313` | the package for a specific interpreter: `py312`, `py313` or `py314`, with a `-mpi` suffix for the MPI build | | `nix run` | a Python interpreter with `monoprop` importable | @@ -122,6 +122,46 @@ from the Python source distribution. compiles the nanobind extension in editable mode. Re-run the sync command whenever the dependency graph or the C++ sources change. +### Build options + +The build is configured through a few package options, declared in +`[tool.scikit-build.config-setting]` in `pyproject.toml`. Each one can be set +as a PEP 517 config-setting or through an environment variable: + +| Config-setting | Environment variable | Values | Effect | +| --- | --- | --- | --- | +| `monoprop.mpi` | `monoprop_ENABLE_MPI` | bool (default off) | MPI parallelization; also adds mpi4py to the build requirements and appends `-mpi` to the build tree | +| `monoprop.sanitizer` | `monoprop_SANITIZER` | `none` (default), `asan-ubsan`, `tsan` | sanitizer profile; also selects the build type (`AsanUbsan`, `Tsan`) and thus the build tree | +| `monoprop.cxx-unit-tests` | `monoprop_ENABLE_CXX_UNIT_TESTS` | bool (default on) | build the C++ unit test suite | +| `monoprop.arch-flags` | `monoprop_ENABLE_ARCH_FLAGS` | bool (default on, forced off for Debug and sanitizer builds) | architecture-specific compiler flags | +| `monoprop.mpi-test-procs` | `monoprop_MPI_TEST_PROCS` | `;`-separated rank counts | rank counts the C++ MPI tests are registered for | + +Booleans accept `ON`/`OFF`, `true`/`false`, `1`/`0` and similar. These two +commands are equivalent: + +```bash +monoprop_ENABLE_MPI=ON uv sync --all-extras -v +uv sync --all-extras -v --config-settings-package="monoprop:monoprop.mpi=ON" +``` + +In the `uv` workspace, prefer `--config-settings-package="monoprop:..."` over +`--config-settings`, so the setting does not reach the sibling distributions in +`packages/`. With `pip`, pass `-C monoprop.mpi=ON`. A misspelled option is +rejected with a suggestion. + + + The environment variable takes precedence over the config-setting. A stale + `monoprop_ENABLE_MPI=OFF` in your shell silently overrides + `monoprop.mpi=ON`. + + +uv rebuilds monoprop whenever an option changes, by either route: config-settings +are part of its build cache key, and `[tool.uv] cache-keys` in `pyproject.toml` +adds the environment variables above, along with `SKBUILD_BUILD_DIR`, +`SKBUILD_CMAKE_BUILD_TYPE` and `SKBUILD_CMAKE_DEFINE`. Switching variants +therefore needs no `--reinstall-package` or `--no-cache`. Any other CMake cache +variable can still be passed as `cmake.define.=...` or through +`SKBUILD_CMAKE_DEFINE`. ### Without MPI (default) @@ -133,7 +173,8 @@ This produces a single-process build with no MPI dependency. ### With MPI -Set `monoprop_ENABLE_MPI=ON` in the environment: +Turn on the `monoprop.mpi` [build option](#build-options), for example through +the environment: ```bash monoprop_ENABLE_MPI=ON uv sync --all-extras -v @@ -142,49 +183,28 @@ monoprop_ENABLE_MPI=ON uv sync --all-extras -v It works the same with `pip` when installing from a checkout: ```bash -monoprop_ENABLE_MPI=ON pip install . +pip install . -C monoprop.mpi=ON ``` -That one variable does two things, matched by the -`[[tool.scikit-build.overrides]]` block in `pyproject.toml`: it sets the -`monoprop_ENABLE_MPI` CMake option, and it adds `mpi4py` to the build -requirements. mpi4py is a build-time input — the bindings need its headers to -hand an MPI communicator across the nanobind boundary — but only for MPI -builds, so it is injected per-build through PEP 517's -`get_requires_for_build_wheel` rather than sitting in `build-system.requires` -where every build would pay for it. +That one option does three things. It sets the `monoprop_ENABLE_MPI` CMake +option, and, through the `[[tool.scikit-build.overrides]]` block in +`pyproject.toml`, it adds `mpi4py` to the build requirements and moves the build +to its own tree, `build/editable/Release-mpi`. - - Because it is an environment variable, it does not participate in uv's build - cache key. Add `--reinstall-package monoprop --no-cache` when flipping MPI on - or off in an existing environment, or uv may reuse the previous build. - - -#### Driving it with config-settings instead - -A scikit-build-core override can only match environment variables, never -config-settings, so `cmake.define` alone will not pull in mpi4py — the build -would fail at configure time with a message telling you as much. Pass both -settings to use this route: - -```bash -uv sync --all-extras -v \ - --config-settings=cmake.define.monoprop_ENABLE_MPI=ON \ - --config-settings=build.requires=mpi4py>=4.1.0 -``` - -In a workspace, prefer the per-package form so the settings do not reach the -sibling distributions in `packages/`: +mpi4py is a build-time input — the bindings need its headers to hand an MPI +communicator across the nanobind boundary — but only for MPI builds, so it is +injected per-build through PEP 517's `get_requires_for_build_wheel` rather than +sitting in `build-system.requires` where every build would pay for it. -```bash -uv sync --all-extras -v \ - --config-settings-package="monoprop:cmake.define.monoprop_ENABLE_MPI=ON" \ - --config-settings-package="monoprop:build.requires=mpi4py>=4.1.0" -``` +The separate tree means switching between the serial and MPI variants rebuilds +incrementally instead of reconfiguring a shared tree from scratch. It is a +directory suffix rather than a build type, so the build keeps CMake's Release +flags. -The cost of this route is that the mpi4py pin is repeated on the command line -instead of living once in `pyproject.toml`; the environment switch is preferred -for that reason. +Setting the CMake option directly (`cmake.define.monoprop_ENABLE_MPI=ON` or +`SKBUILD_CMAKE_DEFINE`) bypasses that override, so mpi4py is not provisioned +and the build fails at configure time with a message telling you as much. Use +`monoprop.mpi` instead. ### Verify the install @@ -248,9 +268,16 @@ ctest --test-dir build/editable/Debug ### Sanitizer trees -The QA workflow builds two sanitizer profiles, selected with `monoprop_SANITIZER` -(`none`, `asan-ubsan`, or `tsan`). Both require Linux and either GCC or Clang; -CI uses the default system GCC, which is what builds the released wheels. +The QA workflow builds two sanitizer profiles, selected with the +`monoprop.sanitizer` [build option](#build-options) (`none`, `asan-ubsan`, or +`tsan`). Each profile has its own build type, `AsanUbsan` or `Tsan`, so it +builds into its own tree next to `build/editable/Release` (with an `-mpi` +suffix for an MPI build). Both require Linux and +either GCC or Clang; CI uses the default system GCC, which is what builds the +released wheels. + +Export `monoprop_SANITIZER` rather than setting it for the build command alone: +the `just` test recipes read it too, to find the tree. Sanitizer trees intentionally skip the `_core.pyi` typing stubs — generating them means importing the instrumented extension, which is not worth arranging for a @@ -259,19 +286,10 @@ tree that is never packaged. #### ASan + LSan + UBSan ```bash -SKBUILD_CMAKE_BUILD_TYPE=AsanUbsan \ -SKBUILD_CMAKE_DEFINE="monoprop_SANITIZER=asan-ubsan" \ -uv sync --group workspace-test --all-extras --reinstall-package monoprop --no-cache -v +export monoprop_SANITIZER=asan-ubsan +just build --group workspace-test ``` -`--reinstall-package monoprop --no-cache` matters locally. uv decides whether to -rebuild from the `cache-keys` in `pyproject.toml`, which hash *source files* — not -environment variables. So if you already have an editable install, switching only -the build type leaves the sources unchanged, `uv sync` does nothing, and -`build/editable/AsanUbsan` is never created (the `ctest` below then fails with -"No such file or directory"). CI omits these flags because each job starts on a -clean runner with nothing installed. - The C++ test binary is fully instrumented, so every check applies to it, including leak detection: @@ -314,9 +332,8 @@ The QA workflow runs that recipe on failure. #### ThreadSanitizer ```bash -SKBUILD_CMAKE_BUILD_TYPE=Tsan \ -SKBUILD_CMAKE_DEFINE="monoprop_SANITIZER=tsan" \ -uv sync --group workspace-test --all-extras --reinstall-package monoprop --no-cache -v +export monoprop_SANITIZER=tsan +just build --group workspace-test just test-cpp-tsan ``` @@ -333,12 +350,13 @@ Python API under TSan would require a TSan-instrumented CPython. - Use `just code-coverage` for the coverage build. - Use `just test-cpp` and `just test-cpp-mpi` for the two C++ legs, or - `ctest --test-dir build/editable/Release -L mpi-2` (or `-L mpi-4`) to filter further. + `ctest --test-dir build/editable/Release-mpi -L mpi-2` (or `-L mpi-4`) to filter further. - Use `just build [uv sync args…]` when you want the build CI performs. It adds `--all-extras` and, for a serial build, drops the `mpi` extra — importing `mpi4py.MPI` without an MPI runtime raises. `monoprop_ENABLE_MPI` drives all of it, so `monoprop_ENABLE_MPI=ON just build --group workspace-test` is the MPI - build and a bare `just build` is the serial one. + build and a bare `just build` is the serial one. The recipes read the + environment variables, not config-settings, so select variants that way here. - The platform packages the build needs are listed in `tools/packages/`, one per line: `apt.txt` / `brew.txt`, plus `apt-mpi.txt` / `brew-mpi.txt` for an MPI build. `.github/actions/setup` and `.devcontainer/Dockerfile` both install from diff --git a/docs/content/docs/testing.mdx b/docs/content/docs/testing.mdx index e01200a8..b2fda6ca 100644 --- a/docs/content/docs/testing.mdx +++ b/docs/content/docs/testing.mdx @@ -46,18 +46,16 @@ Use `@pytest.mark.mpi(min_size=2)` when a test needs at least two ranks. Under `--with-mpi`, only rank zero reports pytest output. To do it by hand, build with MPI on, then run under `mpiexec` with `--no-sync` -so each rank reuses that build. `--reinstall-package` and `--no-cache` force a -genuine rebuild, since uv does not key its build cache on the environment: +so each rank reuses that build. uv rebuilds on its own when the MPI setting +changes, since the build options are part of its cache key: ```bash -monoprop_ENABLE_MPI=ON \ - uv sync --all-extras --reinstall-package monoprop --no-cache +monoprop_ENABLE_MPI=ON uv sync --all-extras mpiexec --allow-run-as-root -n 2 uv run --no-sync python -m pytest tests --with-mpi ``` -`monoprop_ENABLE_MPI` also provisions mpi4py as a build input; see -[Building from source](/building#with-mpi) for why, and for the -config-settings equivalent. Installing the `mpi` extra alone only provides +`monoprop_ENABLE_MPI` (or the `monoprop.mpi` config-setting) also provisions +mpi4py as a build input; see [Building from source](/building#with-mpi) for why. Installing the `mpi` extra alone only provides mpi4py and does not enable C++ MPI support. Accordingly, the CI matrix has explicit cache-free MPI build lanes on Linux x86-64, Linux ARM64, and macOS; only those lanes run the suite under `mpiexec`. @@ -75,12 +73,12 @@ ctest --test-dir build/editable/Release --output-on-failure ``` With MPI, reuse the MPI-enabled `uv sync` from the Python MPI section above, -then run CTest against the same tree: +then run CTest against its tree. MPI builds use their own tree, `Release-mpi`, +so switching between the variants rebuilds incrementally: ```bash -monoprop_ENABLE_MPI=ON \ - uv sync --all-extras --reinstall-package monoprop --no-cache -ctest --test-dir build/editable/Release --output-on-failure +monoprop_ENABLE_MPI=ON uv sync --all-extras +ctest --test-dir build/editable/Release-mpi --output-on-failure ``` Or simply `just test-mpi`. @@ -91,8 +89,9 @@ The full MPI rank matrix (several rank counts) is driven by a helper script: ctest -S tools/ctest-mpi-matrix.cmake -VV ``` -CI gives every compiler and MPI combination a distinct setup-uv cache suffix, -because uv's cache key does not include build environment variables. +CI gives every compiler and MPI combination a distinct setup-uv cache suffix. uv's +cache key covers the [build options](/building#build-options), but not the compiler +selection (`CXX`). A matrix job reports one check per combination, so its check names change whenever a lane is added or removed. Each matrix workflow therefore ends with a small aggregation diff --git a/flake.nix b/flake.nix index 2917a9e1..4aebf836 100644 --- a/flake.nix +++ b/flake.nix @@ -55,14 +55,14 @@ }; monoprop = pyfinal.callPackage ./nix/monoprop.nix { inherit nanobind_3; - # monoprop's `[build-system] requires` wants >= 1.0.3, newer than + # monoprop's `[build-system] requires` wants >= 1.1.0, newer than # nixpkgs ships. scikit-build-core = scikit-build-core.overridePythonAttrs (_: rec { - version = "1.0.3"; + version = "1.1.0"; src = pyfinal.fetchPypi { pname = "scikit_build_core"; inherit version; - hash = "sha256-pNegWXjuN5dcN3Q1EMiZHi3rzn74OvsKB8DFdv1PFug="; + hash = "sha256-1CH4N6D2ReUbmfwNiwDJdpF4RdlU1gnOawaxGyyUYhs="; }; # The nixpkgs test setup targets the version nixpkgs ships. doCheck = false; diff --git a/justfile b/justfile index 0bb875b9..8697793e 100644 --- a/justfile +++ b/justfile @@ -25,7 +25,12 @@ no_mpi_extra := "--no-extra mpi" mpi_enabled := lowercase(env('monoprop_ENABLE_MPI', 'off')) mpi_extra := if mpi_enabled =~ '^(on|true|yes|1)$' { "" } else { no_mpi_extra } -build_dir := "build/editable" / env('SKBUILD_CMAKE_BUILD_TYPE', 'Release') +# Mirrors the sanitizer and MPI overrides in pyproject.toml, which pick the build tree. + +sanitizer := env('monoprop_SANITIZER', 'none') +build_type := env('SKBUILD_CMAKE_BUILD_TYPE', if sanitizer == 'asan-ubsan' { 'AsanUbsan' } else if sanitizer == 'tsan' { 'Tsan' } else { 'Release' }) +mpi_build_dir := "build/editable" / build_type + "-mpi" +build_dir := if mpi_extra == "" { mpi_build_dir } else { "build/editable" / build_type } mpiexec := "mpiexec --map-by :OVERSUBSCRIBE" # Keep both OpenMPI 4 (ORTE) and 5 (PRRTE) spell oversubscription env-vars. @@ -72,15 +77,16 @@ test-py LABEL='': uv run --no-sync pytest -r aR --durations=50 --durations-min=5.0 \ {{ if LABEL == '' { '' } else { '--junit-xml=pytest-' + LABEL + '.xml -o junit_family=legacy' } }} -test-cpp LABEL='': - ctest --test-dir {{ build_dir }} --output-on-failure --no-tests=error --label-exclude mpi \ +test-cpp LABEL='' BUILD_DIR=build_dir: + ctest --test-dir {{ BUILD_DIR }} --output-on-failure --no-tests=error --label-exclude mpi \ {{ if LABEL == '' { '' } else { '--output-junit ctest-serial-' + LABEL + '.xml' } }} # --no-tests=error: the MPI tests are registered only by an MPI-enabled build, so an -# empty set here means the build was misconfigured. +# empty set here means the build was misconfigured. Only MPI builds have them, so this +# always reads the MPI tree. test-cpp-mpi LABEL='': - ctest --test-dir {{ build_dir }} --output-on-failure --no-tests=error --label-regex mpi \ + ctest --test-dir {{ mpi_build_dir }} --output-on-failure --no-tests=error --label-regex mpi \ {{ if LABEL == '' { '' } else { '--output-junit ctest-mpi-' + LABEL + '.xml' } }} # RANKS is a single integer or a semicolon-separated list (e.g. "1;2;4"); the suite runs @@ -98,14 +104,13 @@ test-py-mpi RANKS='' *PYTEST_ARGS: done # Build MPI-enabled, then run every leg. The C++ -# rank matrix is a build-time setting. +# rank matrix is a build-time setting. The MPI switch is local to the build line, so the +# C++ legs are pointed at the MPI tree explicitly. -test-mpi RANKS='': && (test-py-mpi RANKS) test-cpp test-cpp-mpi +test-mpi RANKS='': && (test-py-mpi RANKS) (test-cpp '' mpi_build_dir) test-cpp-mpi requested_ranks={{ quote(RANKS) }}; \ ranks="${requested_ranks:-${monoprop_MPI_TEST_PROCS:-2}}"; \ - monoprop_ENABLE_MPI=ON {{ uv_sync }} --group workspace-test \ - --reinstall-package monoprop --no-cache \ - --config-settings-package="monoprop:cmake.define.monoprop_MPI_TEST_PROCS=${ranks}" + monoprop_ENABLE_MPI=ON monoprop_MPI_TEST_PROCS="$ranks" {{ uv_sync }} --group workspace-test # Build and run a consumer project against the installed package. @@ -119,8 +124,8 @@ test-find-package BUILD_DIR='build/find-package-smoke': cmake --build "$build_dir" "$build_dir/smoke" -# The sanitizer legs run against a tree built with SKBUILD_CMAKE_BUILD_TYPE=AsanUbsan (or -# Tsan) and the matching monoprop_SANITIZER define. +# The sanitizer legs run against a tree built with monoprop_SANITIZER=asan-ubsan (or tsan) +# exported, which also points build_dir at it. # # Only the C++ binary is fully instrumented, so the option sets differ per leg and cannot # be hoisted to the environment. @@ -394,7 +399,7 @@ bench-mpi LABEL RANKS *MPIARGS: # Rebuild monoprop with MPI enabled (editable). Run once before `just bench-mpi`. bench-build-mpi: - monoprop_ENABLE_MPI=ON {{ uv_sync }} --group bench --reinstall-package monoprop --no-cache + monoprop_ENABLE_MPI=ON {{ uv_sync }} --group bench # Quick sanity run: tiny sizes, skip the slow static benchmarks. bench-smoke: diff --git a/nix/monoprop.nix b/nix/monoprop.nix index a869155d..f928ed0c 100644 --- a/nix/monoprop.nix +++ b/nix/monoprop.nix @@ -76,7 +76,8 @@ buildPythonPackage { setuptools-scm ] # Upstream provisions mpi4py through a scikit-build-core override keyed on the - # `monoprop_ENABLE_MPI` environment variable, which would resolve it from PyPI. + # `monoprop.mpi` setting (environment variable `monoprop_ENABLE_MPI`), which would + # resolve it from PyPI. # The sandbox has no network, so the store copy is supplied here instead and the # variable is deliberately left unset -- see `env` below. ++ lib.optionals withMPI [ mpi4py ]; @@ -107,7 +108,7 @@ buildPythonPackage { "nanobind_DIR=${nanobind_3}/${python.sitePackages}/nanobind/cmake" ] # The CMake option is set directly rather than through upstream's - # `monoprop_ENABLE_MPI` environment switch: that switch also appends mpi4py to + # `monoprop.mpi` setting: that setting also appends mpi4py to # `build.requires`, i.e. a PyPI resolution the sandbox denies. ++ lib.optionals withMPI [ "monoprop_ENABLE_MPI=ON" ] ); diff --git a/pyproject.toml b/pyproject.toml index e3ac2eab..52a6aae4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -2,7 +2,7 @@ requires = [ "nanobind-backend>=1.0.0", "nanobind==3.1.0", - "scikit-build-core>=1.0.3,<2", + "scikit-build-core>=1.1.0,<2", "setuptools-scm>=8", ] build-backend = "scikit_build_core.build" @@ -101,6 +101,15 @@ cache-keys = [ { file = "src/monoprop/bindings/binder.h" }, { file = "src/monoprop/bindings/bindings.cpp.in" }, { file = "tools/*.py" }, + # uv does not hash the environment by default; these select the build variant + { env = "monoprop_ENABLE_MPI" }, + { env = "monoprop_SANITIZER" }, + { env = "monoprop_ENABLE_CXX_UNIT_TESTS" }, + { env = "monoprop_ENABLE_ARCH_FLAGS" }, + { env = "monoprop_MPI_TEST_PROCS" }, + { env = "SKBUILD_BUILD_DIR" }, + { env = "SKBUILD_CMAKE_BUILD_TYPE" }, + { env = "SKBUILD_CMAKE_DEFINE" }, ] @@ -157,13 +166,57 @@ sdist.include = [ ] build.verbose = true build.tool-args = ["-j2"] -# monoprop_ENABLE_MPI is the source-build switch. Overrides can only match -# environment variables, never config-settings, so setting an equivalent CMake -# define directly does not activate any associated build requirements. + +# Package build options: pass as `-C monoprop.=...` or through the bound +# environment variable, which takes precedence. Settings without a default drop +# their CMake define when unset, leaving CMake's own default in charge. +[tool.scikit-build.config-setting."monoprop.mpi"] +help = "Build with MPI parallelization; also adds mpi4py to the build requirements" +type = "bool" +env = "monoprop_ENABLE_MPI" + +[tool.scikit-build.config-setting."monoprop.sanitizer"] +help = "Sanitizer profile: none, asan-ubsan, or tsan; also selects the build type (CMake default: none)" +env = "monoprop_SANITIZER" + +[tool.scikit-build.config-setting."monoprop.cxx-unit-tests"] +help = "Build the C++ unit test suite (CMake default: ON)" +type = "bool" +env = "monoprop_ENABLE_CXX_UNIT_TESTS" + +[tool.scikit-build.config-setting."monoprop.arch-flags"] +help = "Enable architecture-specific compiler flags (CMake default: ON outside Debug and sanitizer builds)" +type = "bool" +env = "monoprop_ENABLE_ARCH_FLAGS" + +[tool.scikit-build.config-setting."monoprop.mpi-test-procs"] +help = "Semicolon-separated MPI rank counts the C++ MPI tests run with" +env = "monoprop_MPI_TEST_PROCS" + +[tool.scikit-build.cmake.define] +monoprop_ENABLE_MPI = { config-setting = "monoprop.mpi" } +monoprop_SANITIZER = { config-setting = "monoprop.sanitizer" } +monoprop_ENABLE_CXX_UNIT_TESTS = { config-setting = "monoprop.cxx-unit-tests" } +monoprop_ENABLE_ARCH_FLAGS = { config-setting = "monoprop.arch-flags" } +monoprop_MPI_TEST_PROCS = { config-setting = "monoprop.mpi-test-procs" } + +# mpi4py is a build input only for MPI builds, so it is requested per build +# rather than listed in build-system.requires. MPI builds get their own tree so +# that switching variants rebuilds incrementally instead of reconfiguring one +# shared tree; a suffix rather than a build type keeps the Release flags. [[tool.scikit-build.overrides]] -if.env.monoprop_ENABLE_MPI = true +if.config-setting."monoprop.mpi" = true build.requires = ["mpi4py>=4.1.0"] -cmake.define.monoprop_ENABLE_MPI = "ON" +build-dir = "build/{state}/{build_type}-mpi" + +# Each sanitizer profile gets its own build type, hence its own build tree. +[[tool.scikit-build.overrides]] +if.config-setting."monoprop.sanitizer" = "^asan-ubsan$" +cmake.build-type = "AsanUbsan" + +[[tool.scikit-build.overrides]] +if.config-setting."monoprop.sanitizer" = "^tsan$" +cmake.build-type = "Tsan" [tool.setuptools_scm] From d1cbf5393699e0a3126126bf57878f1534c0a527 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roberto=20Di=20Remigio=20Eik=C3=A5s?= Date: Mon, 26 Oct 2026 12:36:52 +0100 Subject: [PATCH 2/2] =?UTF-8?q?fix(build):=20=F0=9F=90=9B=20restore=20opti?= =?UTF-8?q?on=20defaults=20when=20a=20build=20option=20is=20unset?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The persistent build tree keeps the last CMake cache value when a config-setting is unset and drops its define, so unsetting cxx-unit-tests, arch-flags or mpi-test-procs did not restore the CMake default. Give these settings explicit defaults mirroring CMake, so a define is always passed. arch-flags can default to true because cmake_dependent_option still forces it off for Debug and sanitizer builds. The mpi and sanitizer settings select their own build tree and need no default. Also add SKBUILD_CMAKE_ARGS to the uv cache keys, so changing it triggers a rebuild now that the syncs no longer pass --no-cache. Assisted-by: Pi:claude-opus-5-5 --- docs/content/docs/building.mdx | 10 ++++++++-- pyproject.toml | 16 ++++++++++++---- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/docs/content/docs/building.mdx b/docs/content/docs/building.mdx index 96aabe1c..81052457 100644 --- a/docs/content/docs/building.mdx +++ b/docs/content/docs/building.mdx @@ -134,7 +134,7 @@ as a PEP 517 config-setting or through an environment variable: | `monoprop.sanitizer` | `monoprop_SANITIZER` | `none` (default), `asan-ubsan`, `tsan` | sanitizer profile; also selects the build type (`AsanUbsan`, `Tsan`) and thus the build tree | | `monoprop.cxx-unit-tests` | `monoprop_ENABLE_CXX_UNIT_TESTS` | bool (default on) | build the C++ unit test suite | | `monoprop.arch-flags` | `monoprop_ENABLE_ARCH_FLAGS` | bool (default on, forced off for Debug and sanitizer builds) | architecture-specific compiler flags | -| `monoprop.mpi-test-procs` | `monoprop_MPI_TEST_PROCS` | `;`-separated rank counts | rank counts the C++ MPI tests are registered for | +| `monoprop.mpi-test-procs` | `monoprop_MPI_TEST_PROCS` | `;`-separated rank counts (default `2;4`) | rank counts the C++ MPI tests are registered for | Booleans accept `ON`/`OFF`, `true`/`false`, `1`/`0` and similar. These two commands are equivalent: @@ -155,10 +155,16 @@ rejected with a suggestion. `monoprop.mpi=ON`. +The build tree persists between syncs, and so does its CMake cache. The +`cxx-unit-tests`, `arch-flags` and `mpi-test-procs` options therefore always pass +their default, so unsetting one restores it rather than keeping the last value. +A CMake variable passed through `cmake.define` has no such default: once set, it +keeps its cached value until you set it again or delete the build tree. + uv rebuilds monoprop whenever an option changes, by either route: config-settings are part of its build cache key, and `[tool.uv] cache-keys` in `pyproject.toml` adds the environment variables above, along with `SKBUILD_BUILD_DIR`, -`SKBUILD_CMAKE_BUILD_TYPE` and `SKBUILD_CMAKE_DEFINE`. Switching variants +`SKBUILD_CMAKE_ARGS`, `SKBUILD_CMAKE_BUILD_TYPE` and `SKBUILD_CMAKE_DEFINE`. Switching variants therefore needs no `--reinstall-package` or `--no-cache`. Any other CMake cache variable can still be passed as `cmake.define.=...` or through `SKBUILD_CMAKE_DEFINE`. diff --git a/pyproject.toml b/pyproject.toml index 52a6aae4..fe73bde1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -108,6 +108,7 @@ cache-keys = [ { env = "monoprop_ENABLE_ARCH_FLAGS" }, { env = "monoprop_MPI_TEST_PROCS" }, { env = "SKBUILD_BUILD_DIR" }, + { env = "SKBUILD_CMAKE_ARGS" }, { env = "SKBUILD_CMAKE_BUILD_TYPE" }, { env = "SKBUILD_CMAKE_DEFINE" }, ] @@ -168,8 +169,11 @@ build.verbose = true build.tool-args = ["-j2"] # Package build options: pass as `-C monoprop.=...` or through the bound -# environment variable, which takes precedence. Settings without a default drop -# their CMake define when unset, leaving CMake's own default in charge. +# environment variable, which takes precedence. An unset setting drops its CMake +# define, but the persistent build tree keeps the last cached value, so CMake's +# default only applies to a fresh tree. Settings whose build tree is shared +# between values therefore mirror the CMake default here, so that unsetting them +# restores it. The mpi and sanitizer settings select their own build tree. [tool.scikit-build.config-setting."monoprop.mpi"] help = "Build with MPI parallelization; also adds mpi4py to the build requirements" type = "bool" @@ -180,17 +184,21 @@ help = "Sanitizer profile: none, asan-ubsan, or tsan; also selects the build typ env = "monoprop_SANITIZER" [tool.scikit-build.config-setting."monoprop.cxx-unit-tests"] -help = "Build the C++ unit test suite (CMake default: ON)" +help = "Build the C++ unit test suite" type = "bool" +default = true env = "monoprop_ENABLE_CXX_UNIT_TESTS" [tool.scikit-build.config-setting."monoprop.arch-flags"] -help = "Enable architecture-specific compiler flags (CMake default: ON outside Debug and sanitizer builds)" +help = "Enable architecture-specific compiler flags; forced off in Debug and sanitizer builds" type = "bool" +# cmake_dependent_option forces it off where unsupported, whatever this passes +default = true env = "monoprop_ENABLE_ARCH_FLAGS" [tool.scikit-build.config-setting."monoprop.mpi-test-procs"] help = "Semicolon-separated MPI rank counts the C++ MPI tests run with" +default = "2;4" env = "monoprop_MPI_TEST_PROCS" [tool.scikit-build.cmake.define]