py-proteus: New package - #6000
Conversation
Proteus (proteustoolkit.org): PDE/multiphysics toolkit built around PETSc for parallel linear/nonlinear solves. This first pass deliberately omits Chrono (rigid-body/FSI coupling) and SCOREC/PUMI (parallel mesh adaptation) support: both are optional at the proteus code level, and neither has an upstream Spack package yet (pychrono in particular would be a separate, nontrivial packaging effort). Sources from cekees/proteus's torino_narwhal branch rather than erdc/proteus's tagged releases: the Chrono/SCOREC-optional support this "no chrono" package design depends on (the PROTEUS_SKIP_PUMI_CHRONO env var, and a try/except around an otherwise-unconditional Chrono config-file lookup) hasn't been released upstream yet. Revisit tracking erdc/proteus tags once that support lands there. Depends on two other fixes proposed alongside this PR (not yet merged): xtensor-python@0.28.0 (this branch/PR) and xtensor's xsimd upper bound for 0.26.0:0.27 (this branch/PR) -- xtensor-python was previously stuck at 0.23.1, and xtensor's own xsimd constraint was too permissive for the 0.26/0.27 line. Built and verified working end-to-end (spack install + import proteus + petsc4py Comm.init()) against a from-scratch toolchain, including building openmpi/petsc/etc. from source rather than relying on any external MPI.
setup.py gated MeshAdaptPUMI.MeshAdapt and mbd.CouplingFSI behind one PROTEUS_SKIP_PUMI_CHRONO switch. Chrono stays skipped unconditionally (pychrono has no upstream Spack package), but PUMI does have one now (`pumi`), so the new `scorec` variant (default off) depends on pumi+zoltan+shared, zoltan+parmetis~fortran, and parmetis, points SCOREC_DIR/ZOLTAN_DIR/PARMETIS_DIR at them, and applies split-pumi-chrono-skip.patch to split the single skip switch into independent PROTEUS_SKIP_PUMI/PROTEUS_SKIP_CHRONO ones so PUMI can be enabled while Chrono stays off. pumi needs +shared explicitly: it defaults to static libs without -fPIC, which can't link into MeshAdaptPUMI.MeshAdapt (a shared Python extension). zoltan needs ~fortran: its Fortran interface is unused here and its shared lib leaves unresolved libgfortran symbols on this toolchain when built with +fortran. Verified end to end: py-proteus+scorec builds and proteus.MeshAdaptPUMI.MeshAdapt imports and resolves its shared libraries correctly at runtime. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MraeTXJFpMRZJUdYthLxyg
…_narwhal cekees/proteus@torino_narwhal now natively splits PROTEUS_SKIP_PUMI_CHRONO into independent PROTEUS_SKIP_PUMI/PROTEUS_SKIP_CHRONO switches (pushed directly to that branch), so the Spack-side patch is redundant -- and would otherwise break the next fetch, since its "before" context (the single combined switch) no longer exists in the source. Verified py-proteus+scorec builds against the new commit with no patch and MeshAdaptPUMI.MeshAdapt still imports correctly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MraeTXJFpMRZJUdYthLxyg
Comments were written narrating the investigation (what was tried, what was validated when, "this session"); condense each down to the non-obvious fact a maintainer actually needs -- the constraint or workaround itself, not the story behind finding it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MraeTXJFpMRZJUdYthLxyg
…brary triangle 1.6's own upstream makefile hardcodes CSWITCHES with -DLINUX unconditionally, regardless of the actual build platform. triangle.c only consults that macro (alongside the mutually-exclusive -DCPU86) to twiddle the legacy x87 FPU's precision-control register on old x86 hardware -- irrelevant on Apple Silicon and unnecessary on modern x86_64 -- but the macro also gates an unconditional #include <fpu_control.h>, a glibc/Linux- only header that doesn't exist on macOS. Building with the stock makefile there failed outright with "fatal error: 'fpu_control.h' file not found". Override CSWITCHES on Darwin to drop -DLINUX; Linux builds are unaffected. Also: this package previously only installed the CLI executables (triangle, showme), nothing a dependent could compile/link against. Consumers like proteus link directly against triangle's C API (a single object file built from triangle.c with -DTRILIBRARY, per the upstream makefile's own "trilibrary" target) rather than shelling out to the CLI. Without an installed header+library, proteus's own build failed with "fatal error: 'triangle.h' file not found". Build and install both, matching what proteus's config/default.py expects: $TRIANGLE_DIR/include/ triangle.h and $TRIANGLE_DIR/lib/libtri.a. Confirmed via a real `spack install py-proteus` build.
Two related but distinct macOS linking bugs, both only visible once a
consumer actually tries to load the built library:
1. The %gcc branch appends a bare -lgfortran with no matching -L. This
silently works when gfortran is a system/Homebrew package (its runtime
lib sits on a default linker search path) but fails against Spack's own
from-source-built GCC, whose runtime libs live in their own package
prefix: "ld: library 'gfortran' not found". Fixed by asking the Fortran
compiler itself where its own libgfortran lives
(`gfortran -print-file-name=libgfortran.dylib`) rather than guessing a
layout, so this works regardless of the actual gcc package Spack picked.
2. This package fakes a shared library via a compiler-as-archiver trick
(`--with-ar="$(CXX) -shared $(LDFLAGS) -o"`, since Zoltan's own build
system only ever knows how to archive into a .a) and then renames the
resulting file from .a to .dylib in solib_install(). The rename is
file-system only -- it doesn't touch the dylib's own embedded
LC_ID_DYLIB, which clang baked in as the literal, path-less string
"libzoltan.a" (the link command's own -o argument) since no explicit
-install_name was ever passed. Any consumer linked against this library
afterward (pumi's libapf_zoltan.dylib, confirmed via a real
`spack install py-proteus+scorec` build) captures that bogus self-
reference as *its own* dependency, and fails to dlopen ("Library not
loaded: libzoltan.a") however correct its own rpath otherwise is. Fixed
by correcting the dylib's self-declared ID via `install_name_tool -id`
right after the rename, to match what it now actually is.
…est git override SuperLU's fill-reducing ordering calls METIS_NodeND, but metis was previously only pulled in transitively via parmetis, and only when +scorec. Spack only wires up the rpath/link-path for a package's *direct* depends_on()s, so the base (~scorec) build's superluWrappers extension failed at import time with "symbol not found in flat namespace '_METIS_NodeND'" (confirmed via a real `spack install py-proteus` build). The actual fix for the missing link flags lives in proteus's own setup.py (config/default.py + setup.py, torino_narwhal branch) -- this package now just needs to depend on metis directly and export METIS_DIR unconditionally so proteus's build can find it, matching the existing per-dependency *_DIR pattern already used here for superlu/triangle/etc.
…ll paths
The --download-proteus/HPC and pip paths both configure PETSc with
--download-hypre --download-superlu_dist; this Spack recipe's plain
depends_on("petsc+mpi") left both disabled by default, silently narrowing
which of proteus's solver code paths actually get exercised rather than
producing an obvious failure.
Confirmed via two full pytest runs against a real `spack install
py-proteus+scorec` build: ~hypre~superlu-dist showed 35 failed/387 passed
(mostly AMG-based tests -- test_operator_shells, test_stokesDC_TH_2D,
test_bochev_pressure_stabilization, the amg_* cases in
test_nse_RANS2P_step -- plus a few needing a parallel direct solver);
rebuilding with +hypre+superlu-dist brought that down to 17 failed/405
passed, closing every one of the AMG/operator-shell failures. The
remaining 17 (vs. the 6 known pre-existing failures other paths show) look
like real numerical differences from this Spack build's specific
hypre@3.1.0/superlu-dist@9.2.1 versions rather than a missing capability,
and haven't been chased further.
…X11 paths pumi: mds/apfMDS.cc's getFaceIdInRegion()/getEdgeIdInFace() helpers read the "_vert_id" mesh tag (created as an 8-byte long via createLongTag) with getIntTag() into a 4-byte int -- SCOREC's generic tag storage does an unconditional memcpy(dest, storage, tag->bytes) with no type/size check, so every call is an 8-byte-into-4-byte stack buffer overflow. Confirmed via gdb on aarch64 (petsc/download-proteus-support session): crashes every PUMI-mesh-generation test through reconstructFromProteus2 -> derive2DMdlFromManifold. Already fixed for PETSc's own --download-scorec via scorec.py in the gitlab.com/cekees/petsc download-proteus-support fork; this is the Spack-side equivalent so py-proteus+scorec gets the same fix, since Spack's pumi recipe clones pristine upstream SCOREC/core independently. Also replaces the silent "return 12; // Should give segmentation fault" fallback with an explicit assertion. triangle: showme's build hardcoded CSWITCHES to a legacy -I/usr/X11R6/include path that doesn't exist when X11 comes from Spack's own libx11 dependency -- the generic build system used here doesn't auto-inject dependency include/lib paths the way Autotools/CMakePackage do. Wire spec["libx11"]/spec["xproto"] prefixes into CSWITCHES explicitly instead (Xlib.h #includes X11/X.h, which comes from xproto, only pulled in transitively before this fix -- add a direct build dep on it too), and apply on every platform rather than only Darwin, keeping -DLINUX on Linux where it belongs.
|
@cekees. I just wanted to test how Copilot review does on this PR. If it asks for change, check carefully, don't apply blindly, it might hallucinate still :) |
There was a problem hiding this comment.
Pull request overview
Adds a new Spack py-proteus package (tracking Proteus’ torino_narwhal branch) and includes supporting packaging fixes to enable successful builds on more toolchains/platforms (notably Darwin) and to provide required linkable artifacts for downstream consumers.
Changes:
- Add a new
py-proteusPython package with a+scorecvariant and extensive dependency/env wiring for Proteus’ build system. - Update
triangleto handle Darwin builds and install a linkabletriangle.h+libtri.afor consumers. - Update
zoltanto improve shared-library linking on macOS toolchains (libgfortran discovery and dylib install-name fixups).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| repos/spack_repo/builtin/packages/zoltan/package.py | Improve macOS/shared-library robustness (libgfortran search path + dylib install_name). |
| repos/spack_repo/builtin/packages/triangle/package.py | Add Darwin-specific build flag handling and install triangle headers/library for link-time consumers. |
| repos/spack_repo/builtin/packages/py_proteus/package.py | New Proteus Python package with variants, dependencies, and environment configuration for builds. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| cswitches = "-O -I/usr/X11R6/include -L/usr/X11R6/lib" | ||
| if spec.satisfies("platform=darwin"): | ||
| make("CSWITCHES=" + cswitches) | ||
| else: | ||
| make() | ||
| mkdirp(prefix.bin) | ||
|
|
||
| install("triangle", prefix.bin) | ||
| install("showme", prefix.bin) | ||
|
|
||
| # This package previously only installed the CLI executables above, | ||
| # nothing a dependent could compile/link against -- but consumers | ||
| # like proteus link directly against triangle's C API (a single | ||
| # object file, triangle.o, built from triangle.c with -DTRILIBRARY | ||
| # per the upstream makefile's own "trilibrary" target) rather than | ||
| # shelling out to the CLI. Without an installed header+library, | ||
| # proteus's own build fails with "fatal error: 'triangle.h' file not | ||
| # found" (confirmed via a real `spack install py-proteus` build). | ||
| # Build and install both, matching what proteus's config/default.py | ||
| # expects: $TRIANGLE_DIR/include/triangle.h and | ||
| # $TRIANGLE_DIR/lib/libtri.a. | ||
| make("CSWITCHES=" + cswitches, "trilibrary") |
Co-authored-by: Satish Balay <balay@mcs.anl.gov>
|
@spackbot fix style |
|
Let me see if I can fix that for you! |
|
I was able to run spack style --fixrepos/spack_repo/builtin/packages/petsc/package.py
repos/spack_repo/builtin/packages/pumi/package.py
repos/spack_repo/builtin/packages/py_proteus/package.py
repos/spack_repo/builtin/packages/triangle/package.py
repos/spack_repo/builtin/packages/zoltan/package.py
�[1;34m==> �[0mrunning ruff format
2 files reformatted, 3 files left unchanged
�[1;34m==> �[0mrunning ruff check
All checks passed!
�[1;34m==> �[0mstyle checks passed
I've updated the branch with style fixes. |
There was a problem hiding this comment.
@balay and @cekees, as this PR is under construction, and you're intensively reviewing and improving the recipe, I'd like to prevent that GitLab CI pipelines already start building its predefined pipelines before the PR is fully ready.
The GitLab CI pipelines won't build the package anyway because it only tests the predefined pipelines defined in the CI stacks below the .ci directory.
- For this reason, I'd like to set draft status until @balay is happy with the package and approves it.
- Just before the final commit to make the fully ready, click "Ready for review" button below the list of checks on GitHub to enable running the GitLab CI pipelines and then push the a final git push (or rebase/merge from develop) to trigger the CI pipelines or write a message containting
@spackbotrun pipelineand mention/ping me or another maintainer for approval as well (and I'd also set the PR as ready to enable the CI pipelines so it can be merged/auto-merged afterwards.) - In case you set it to ready for review earlier, it is fine, I just wanted to not run the GitLab pipelines too often when it serves no purpose yet.
|
CI was flagging issues - and I was able to fix some of them. Likely the outstanding issue is the dependency on #6116 I think the PETSc changes in this PR are good to go. [and can approve (this part) - when the PR changes are complete] |
…pfMDS.cc int/long fix)
Project Chrono, the multi-physics simulation engine. No Spack package existed
for it, which is why proteus's py-proteus recipe sets PROTEUS_SKIP_CHRONO=1 and
disables Chrono support outright.
Builds the C++ libraries, and with +python the SWIG-generated pychrono module,
which Chrono produces from the same tree rather than shipping separately.
Optional modules that pull dependencies unrelated to multibody dynamics
(cascade, irrlicht, matlab, postprocess, ros, vehicle) are off.
Verified by building py-proteus against it and running proteus's suite. With
Chrono disabled that pathway reports 508 passed / 6 failed / 2 errors from 514
collected; with this package it reports 517 passed / 0 failed / 1 error from 517
collected. Every chrono-dependent test passes -- AddedMass 2D/3D, four FSI
cases, and test_mbd_chrono, whose three tests account for 514 -> 517. The
remaining error is unrelated (a dead `import cython` in proteus's own
test_bodydynamics.py, since fixed upstream).
Four things here are not obvious, and each cost a build to find:
- depends_on("c") as well as "cxx". Chrono compiles C sources
(chrono_thirdparty/libstl/stlfile.c); without it cmake fails immediately
with "[spack cc]: Error: SPACK_CC_* variables not set".
- NOT extends("python"). Chrono installs pychrono into share/chrono/python,
never into site-packages -- an installed prefix contains only include/ lib/
share/ importer_blender/. extends() promises spack a layout that does not
exist, and the module is then simply unimportable. Setting CH_INSTALL_PYTHON
does not move it either (tried against 10.0.0). PYTHONPATH is exported for
this package and its dependents instead, which is what PETSc's own proteus
build does. Dependents must declare chrono a run dependency for that to
apply.
- CH_DEBUG_POSTFIX forced empty; Chrono otherwise appends "_d" to every
library name in a Debug build while consumers link the unsuffixed name.
- Python3_INCLUDE_DIR/Python3_LIBRARY set explicitly, because Chrono's
find_package(Python3 COMPONENTS Development) has been seen to populate only
Python3_EXECUTABLE, after which the SWIG module links without libpython and
fails on every CPython symbol. On darwin the module also needs
-undefined dynamic_lookup: chrono_python's _core target neither links
-lpython nor passes it, which is fine on Linux (resolved at dlopen) but a
hard link error for a macOS bundle.
Also adds the <type_traits> include ChClassFactory.h relies on transitively. It
uses std::enable_if/is_polymorphic/is_abstract but includes only <cstdio>
<string> <functional> <typeindex> <unordered_map> <memory>; libstdc++ and
Apple's libc++ still provide it transitively, newer libc++ does not, and
Chrono_core then fails with "no member named 'is_polymorphic' in namespace
'std'". Done as a filter_file so it becomes a no-op once upstream adds it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The recipe set PROTEUS_SKIP_CHRONO=1 with the comment "Chrono (pychrono)
currently has no upstream Spack package and stays disabled". There is one now
(merged into this branch), so proteus can be built with its Chrono coupling.
Three changes, each needed:
- depends_on("chrono+python") with run type. The run type is not incidental:
pychrono installs into share/chrono/python rather than site-packages, so
chrono exports PYTHONPATH via setup_dependent_run_environment, and spack
only applies that for run dependencies. Omit it and pychrono is unimportable
at test time even though everything builds.
- CHRONO_DIR in the build environment. proteus/config/default.py's
get_flags("chrono") reads $CHRONO_DIR/{include,lib} and pulls
CHRONO_CXX_FLAGS out of lib/cmake/Chrono/ChronoConfig.cmake; dropping the
skip flag alone is not sufficient.
- PROTEUS_SKIP_CHRONO removed.
Measured on osx-arm64, proteus at d6341f48, same command either way:
without chrono 508 passed / 6 failed / 2 errors / 514 collected
with chrono 532 passed / 0 failed / 0 errors / 532 collected
The six failures were AddedMass 2D/3D and four FSI cases, all needing
proteus.mbd.CouplingFSI; one error was test_mbd_chrono, whose three tests
account for 514 -> 517. (The second error was a dead `import cython` in
proteus's own test_bodydynamics.py, fixed upstream since; its 15 tests are the
517 -> 532.)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three calls were wrapped across lines that fit inside spack's 99-column limit, which ruff-format rejects. Formatting only, no semantic change; applied with `spack style --fix`. The prechecks/style job failed on this in 13s. The F403/F405 star-import warnings that also appear locally are not the cause -- they fire on every spack package using `from spack.package import *`, including py_proteus's own pre-existing depends_on lines. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
proteus shells out to the gmsh executable as a mesh generator. It needs
neither gmsh's FLTK GUI nor MED file support, but both are gmsh defaults
and both pull in dependencies that fail to build on current toolchains:
+fltk -> mesa@25.0.5, whose vendored src/c11/threads.h defines once_flag
and call_once and so conflicts with glibc 2.43's own <threads.h>
("conflicting types for 'once_flag'; have 'pthread_once_t'")
+med -> med@5.0.0, which passes an incompatible pointer as argument 5 of
H5Literate2 and is rejected by gcc >= 14, where
-Wincompatible-pointer-types is an error
Excluding both drops them from the DAG entirely and fixes the build on
Ubuntu 26.04 / gcc 15.2 / glibc 2.43, where py-proteus+pumi previously could
not be built at all. It also removes mesa, one of the heavier items in the
tree, from every other platform's build.
Verified on Ubuntu 26.04 LTS, gcc 15.2.0, glibc 2.43: spack install
py-proteus+pumi completes, the gmsh executable is present and functional,
and the proteus test suite reports 532 passed, 0 failed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
TEMPORARY, pending upstream: this carries a fix for a package we do not own,
so that py-proteus can be built on macOS x86_64 at all. Drop it once
spack/spack-packages takes the change.
build() ran `make` with no target, which is OpenBLAS's default `all`:
all :: libs netlib $(RELA) tests shared
so the test suite was built and executed by an ordinary `spack install
openblas`. That defeats the recipe's own check_build/check_install hooks,
both guarded by @on_package_attributes(run_tests=True), which are the
intended way to opt into testing -- `--test` added a second test run rather
than gating the first.
It also makes any test failure a hard build failure. On macOS x86_64
(TARGET=SKYLAKEX NO_AVX512=1 DYNAMIC_ARCH=1, apple-clang 21 + gfortran 16.2):
TEST 1/125 min:smin_negative make[1]: *** [Makefile:89: run_test] Bus error: 10
make: *** [Makefile:178: tests] Error 2
The library itself compiles cleanly; only its test binary crashes.
Building `libs netlib shared` is `all` minus `tests` (and minus $(RELA),
empty unless re-lapack is enabled). Verified on the affected host: openblas
installs (10m53s), the full py-proteus+pumi tree then builds with no further
failures (30m33s), and the proteus suite reports 532 passed, 0 failed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fixes a regression in the previous commit. Passing "libs netlib shared" as
three goals to a single make lets them run concurrently, which races:
ar: ../libopenblas-r0.3.34.a: error reading zlatm5.o: file truncated
make[1]: *** [../Makefile.tail:95: commonlibs] Error 1
on linux-alderlake/gcc 15.2. This is exactly the hazard build()'s docstring
names -- "Override 'make all' with sequential builds due to race conditions"
-- and the reason the method exists at all. macOS happened to survive it,
which is why the first version looked fine.
Invoking each target in its own make preserves the ordering while still
excluding "tests" from the build, which was the point of the change.
Verified on linux-alderlake/gcc 15.2, where the three-goal form failed:
openblas installs cleanly (6m35s, exit 0).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Satish Balay <balay@mcs.anl.gov>
- make pumi package.py diff match the upstream PR - properly implement pumi and chron variants - remove triangle library build, which proteus doesn't need
|
@spackbot fix style |
|
Let me see if I can fix that for you! |
|
I was able to run spack style --fixrepos/spack_repo/builtin/packages/chrono/package.py
repos/spack_repo/builtin/packages/openblas/package.py
repos/spack_repo/builtin/packages/petsc/package.py
repos/spack_repo/builtin/packages/pumi/package.py
repos/spack_repo/builtin/packages/py_proteus/package.py
repos/spack_repo/builtin/packages/triangle/package.py
repos/spack_repo/builtin/packages/zoltan/package.py
�[1;34m==> �[0mrunning ruff format
1 file reformatted, 6 files left unchanged
�[1;34m==> �[0mrunning ruff check
All checks passed!
�[1;34m==> �[0mstyle checks passed
I've updated the branch with style fixes. |
Review status:
Proteus (proteustoolkit.org): PDE/multiphysics toolkit built around PETSc for parallel linear/nonlinear solves.
@bernhardkaindl this depends on #5826 and #5829 as referenced in the comments.
This first pass deliberately omittted Chrono (rigid-body/FSI coupling) and SCOREC/PUMI (parallel mesh adaptation) support: both are optional at the proteus code level. Now PUMI has an upstream package with a PR that will work for this package. Chrono has a new package added to this PR.
Sources from cekees/proteus's main branch rather than erdc/proteus's tagged releases: the Chrono/SCOREC-optional support this "no chrono" package design depends on (the PROTEUS_SKIP_PUMI_CHRONO or PROTEUS_SKIP_PUMI and PROTEUS_SKIP_CHRONO env vars, and a try/except around an otherwise-unconditional Chrono config-file lookup) hasn't been released upstream yet. Revisit tracking erdc/proteus tags once that support lands there.
Depends on two other fixes proposed alongside this PR (now merged): xtensor-python@0.28.0 (this branch/PR) and xtensor's xsimd upper bound for 0.26.0:0.27 (this branch/PR) -- xtensor-python was previously stuck at 0.23.1, and xtensor's own xsimd constraint was too permissive for the 0.26/0.27 line. Built and verified working end-to-end (spack install + import proteus + petsc4py Comm.init()) against a from-scratch toolchain, including building openmpi/petsc/etc. from source rather than relying on any external MPI.