CMake: keep derived AMReX options in sync on re-configure - #7204
Open
ax3l wants to merge 3 commits into
Open
Conversation
ax3l
force-pushed
the
amrex-cache-staleness-sweep
branch
from
August 26, 2026 22:54
e36b764 to
17e31b7
Compare
Several AMReX options that WarpX derives from its own options were only ever assigned in their "on" branch, or through a non-forcing `set(... CACHE BOOL "")` that is a no-op once the cache entry exists. Re-configuring an existing build directory therefore kept the old value and produced a build that differs from a fresh configure of the same command line: - `CMAKE_BUILD_TYPE=Debug` -> `Release` kept `AMReX_ASSERTIONS` and `AMReX_FPE` on, so a "Release" build still paid for assertions and floating-point exception trapping. - `WarpX_PYTHON=ON` -> `OFF` kept `AMReX_BUILD_SHARED_LIBS=ON`, so AMReX stayed a shared library with no Python bindings asking for it. - `AMReX_ASCENT`, `AMReX_CATALYST`, `AMReX_CONDUIT` and `AMReX_SENSEI` had the same on-only shape. Assign all of them in both directions. `AMReX_BUILD_SHARED_LIBS` is unset rather than set to `OFF`, because `AMReX_INSTALL` below distinguishes "WarpX forced it" from "`BUILD_SHARED_LIBS` decides". Also drop a duplicated `AMReX_PIC` assignment whose condition is a subset of the preceding one. Same class of issue as `AMReX_GPU_RDC`/`AMReX_CUDA_LTO`/`AMReX_IPO` in BLAST-WarpX#7202
Unsetting the cache entry cannot tell a stale value from one that a parent project set deliberately. ImpactX sets it for its Python bindings before adding WarpX/ABLASTR as a subdirectory, so clearing it there made `AMReX_INSTALL` empty and broke the ImpactX Python build with "install(EXPORT "pyAMReXTargets" ...) includes target "pyAMReX_3d" which requires target "amrex_3d" that is not in any export set". Only clear it when WarpX is the top-level project; as a subproject the parent owns the variable.
ax3l
force-pushed
the
amrex-cache-staleness-sweep
branch
from
August 27, 2026 16:06
17e31b7 to
d04f36e
Compare
ax3l
added a commit
to ax3l/impactx
that referenced
this pull request
Sep 5, 2026
`AMReX_PIC`, `ABLASTR_POSITION_INDEPENDENT_CODE` and `AMReX_BUILD_SHARED_LIBS` were only assigned in the "on" branch of `if(ImpactX_PYTHON OR BUILD_SHARED_LIBS)`. Re-configuring an existing build directory with `-DImpactX_PYTHON=OFF` therefore kept the cached `ON` and produced a build that differs from a fresh configure of the same command line: AMReX stayed a shared, position-independent library with no Python bindings asking for it. Assign them in both directions. `AMReX_BUILD_SHARED_LIBS` is unset rather than set to `OFF`, because ABLASTR derives `AMReX_INSTALL` from whether anybody forced it; unsetting restores exactly the state a fresh configure would have. Same class of issue as BLAST-WarpX/warpx#7204
ax3l
added a commit
to ax3l/impactx
that referenced
this pull request
Sep 5, 2026
`AMReX_PIC`, `ABLASTR_POSITION_INDEPENDENT_CODE` and `AMReX_BUILD_SHARED_LIBS` were written to the cache from the "on" branch of `if(ImpactX_PYTHON OR BUILD_SHARED_LIBS)`. Those entries outlive the requirement that created them, so re-configuring an existing build directory with `-DImpactX_PYTHON=OFF` kept the cached `ON` and produced a build that differs from a fresh configure of the same command line: AMReX stayed a shared library with no Python bindings asking for it. Set them as normal variables instead. With `CMP0077` `NEW`, which we already request for the ABLASTR superbuild, the `option()` calls downstream honor a normal variable and skip creating the cache entry, so the requirement applies for exactly the configure that asks for it and leaves nothing behind for the next one. Explicit `-D` values from the caller still take precedence, because they are cache entries and we no longer `FORCE` over them. No `else()` branch: with nothing cached there is nothing to reset, and assigning `OFF` there would override ABLASTR's own `ABLASTR_POSITION_INDEPENDENT_CODE` default of `ON` and change what a fresh non-Python configure builds today. Verified by configuring `ImpactX_PYTHON=ON`, re-configuring the same directory with `ImpactX_PYTHON=OFF`, and comparing against a fresh `ImpactX_PYTHON=OFF` configure. The effective values and the resulting `amrex_3d`/`ablastr_3d` target type and `POSITION_INDEPENDENT_CODE` now match the fresh configure exactly, and both match what `development` produces for a fresh configure. Same class of issue as BLAST-WarpX/warpx#7204 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pg2X154siq9ZHxZqosufjp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Several AMReX options that WarpX derives from its own options were only assigned in their "on" branch, or through a non-forcing
set(... CACHE BOOL "")that is a no-op once the cache entry exists, so re-configuring an existing build directory silently kept the old value. Most visibly, switchingCMAKE_BUILD_TYPEfromDebugtoReleaseleftAMReX_ASSERTIONSandAMReX_FPEon, so a "Release" build still paid for assertions and floating-point exception trapping.Details
AMReX_ASSERTIONS,AMReX_FPECMAKE_BUILD_TYPEDebug→Releasekeeps assertions and FPE trapping onAMReX_BUILD_SHARED_LIBSWarpX_PYTHON,WarpX_LIBON→OFFkeeps AMReX a shared libraryAMReX_ASCENT,AMReX_CATALYST,AMReX_CONDUITWarpX_ASCENT,WarpX_CATALYSTAMReX_SENSEIWarpX_SENSEIThe options fall into two groups, handled differently.
Purely derived values —
AMReX_ASCENT,AMReX_CATALYST,AMReX_CONDUIT,AMReX_SENSEI— are assigned unconditionally from their WarpX counterpart. These stayCACHE INTERNAL, which already impliesFORCE, so the fix for them is a second assignment rather than an addedFORCE.Caller-facing options —
AMReX_ASSERTIONS,AMReX_FPE,AMReX_PIC,AMReX_BUILD_SHARED_LIBS— are set as normal variables instead of cache entries. WithCMP0077 NEW, which we already request here, AMReX'soption()honors a normal variable and skips creating the cache entry, so the derivation re-evaluates on every configure and leaves nothing behind for the next one.AMReX_ASSERTIONSandAMReX_FPEare guarded byif(NOT DEFINED ...), so an explicit-DAMReX_ASSERTIONS=ONis a cache entry that keeps winning;AMReX_PICandAMReX_BUILD_SHARED_LIBSapply unconditionally, since a Python build genuinely requires them.AMReX_INSTALLkeeps working: it is derived fromif(DEFINED AMReX_BUILD_SHARED_LIBS), andDEFINEDis true for a normal variable, so a Python configure still getsAMReX_INSTALL=ONand downstreaminstall(EXPORT ...)is generated. This is what ImpactX's Python bindings need when they add WarpX/ABLASTR as a subdirectory.Internal: drops a duplicated
AMReX_PICassignment whose condition is a strict subset of the preceding one.AMReX_PICitself was not observably stale, sinceABLASTR_POSITION_INDEPENDENT_CODEdefaults toONand keeps the condition true.Two consequences worth knowing. A build directory configured by an earlier version still carries its old cache entries and is not healed by this change — one
--freshclears it permanently. And these options no longer appear inccmake/cmake-guias tweakable knobs in the common case, though-Dstill works and now sticks across re-configures.Testing
Configured, then re-configured the same directory with the opposite settings, and compared each result against a fresh configure of the same command line. Probed the effective values plus the resulting
amrex_3dtargetTYPEandPOSITION_INDEPENDENT_CODE, since the fix intentionally keeps these values out of the cache.AMReX_ASSERTIONSAMReX_FPEamrex_3dRelease+WarpX_PYTHON=OFFSTATIC, PICONDebug+ON→ re-configureRelease+OFFSTATIC, PICONDebug+WarpX_PYTHON=ONSHARED, PICONRelease+OFF→ re-configureDebug+ONSHARED, PICONBoth directions match their fresh reference. A caller's
-DAMReX_ASSERTIONS=ONin aReleasebuild also survives a later re-configure.Also checked the ImpactX superbuild against this branch, since ImpactX sets these options before adding WarpX/ABLASTR as a subdirectory:
ImpactX_PYTHON=ONgives AMReX shared and position-independent withAMReX_INSTALL=ONand noinstall(EXPORT "pyAMReXTargets" ...)error, and itsON→OFFre-configure matches a freshOFFconfigure.Same class of issue as
AMReX_GPU_RDC/AMReX_CUDA_LTO/AMReX_IPOin #7202, and mirrored on the ImpactX side in BLAST-ImpactX/impactx#1637.