Skip redundant HDF5 detection during model CMake configure - #3220
Open
dweindl wants to merge 1 commit into
Open
Conversation
find_dependency(HDF5 COMPONENTS C HL CXX REQUIRED) in AmiciConfig.cmake re-runs FindHDF5.cmake's compiler-wrapper interrogation (spawning h5cc/h5c++ subprocesses) on every single model configure. That result never changes for a given AMICI installation, and on non-Config-mode HDF5 packaging (e.g. Debian/Ubuntu system HDF5) this consistently costs several hundred ms per model, on top of the ABI/toolchain detection every fresh CMake build tree pays anyway. The goal is to speed up importing/building many models in one go, e.g. our own test suite, benchmark collections, or PEtab batch imports, where this per-model overhead otherwise adds up across dozens of independent, freshly-configured build directories - including under pytest-xdist, where models can't share a build directory across parallel workers. Capture the resolved hdf5/hdf5_cpp/hdf5_hl/hdf5_hl_cpp library paths once, when AMICI core itself is built, and bake them into AmiciConfig.cmake as plain imported targets instead of re-deriving them for every model. Falls back to the full find_dependency() search if the cached paths no longer exist on disk. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3220 +/- ##
==========================================
- Coverage 78.59% 77.94% -0.66%
==========================================
Files 318 318
Lines 21090 21090
Branches 1487 1487
==========================================
- Hits 16576 16438 -138
- Misses 4506 4644 +138
Partials 8 8
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
dweindl
marked this pull request as ready for review
August 5, 2026 13:18
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
2 participants
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.
Summary
AmiciConfig.cmakecurrently callsfind_dependency(HDF5 COMPONENTS C HL CXX REQUIRED), which re-runsFindHDF5.cmake's compiler-wrapper interrogation (spawningh5cc/h5c++subprocesses) on every single model configure, even though the result never changes for a given AMICI installation.AmiciConfig.cmakeas plain imported targets (hdf5::hdf5,hdf5::hdf5_cpp,hdf5::hdf5_hl,hdf5::hdf5_hl_cpp) instead of re-deriving them for every model. Falls back to the fullfind_dependency()search if the cached paths no longer exist on disk (e.g. HDF5 was moved/reinstalled since AMICI was built).Test plan
pip install -e python/sdist --no-build-isolation) and confirmed the regeneratedAmiciConfig.cmakehas the baked-in HDF5 paths substituted correctlypython/tests/test_sbml_import.py(model-import/build tests) against the rebuilt core: all pass🤖 Generated with Claude Code