Skip to content

Opt dep cleanup#100

Open
teseoch wants to merge 8 commits into
mainfrom
opt-dep-cleanup
Open

Opt dep cleanup#100
teseoch wants to merge 8 commits into
mainfrom
opt-dep-cleanup

Conversation

@teseoch
Copy link
Copy Markdown
Member

@teseoch teseoch commented May 22, 2026

No description provided.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.50%. Comparing base (22f6767) to head (5691597).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #100   +/-   ##
=======================================
  Coverage   80.50%   80.50%           
=======================================
  Files          49       49           
  Lines        2119     2119           
  Branches      283      283           
=======================================
  Hits         1706     1706           
  Misses        413      413           
Flag Coverage Δ
polysolve 80.50% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors PolySolve’s CMake dependency handling so that optional solver backends are enabled only when their corresponding CMake targets are actually available/linkable, and adds CI “contract” checks to keep that behavior stable across platforms.

Changes:

  • Introduces a new polysolve_optional_dependency CMake helper module and applies it across multiple dependency recipes to probe system availability and gracefully disable missing optional solvers.
  • Adjusts platform/architecture handling (e.g., MKL defaults on ARM) and tightens “only link/define when target exists” logic in the top-level CMakeLists.txt.
  • Adds a new optional-dependency contract-test CMake project and a GitHub Actions workflow to validate target/definition behavior under different dependency-availability scenarios.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
CMakeLists.txt Centralizes optional-solver enablement around target existence, adds ARM detection and MKL default logic, and wires in the new optional-dependency helper.
tests/CMakeLists.txt Renames the unit test executable target and updates link/test discovery wiring accordingly.
tests/cmake/optional_dependencies/CMakeLists.txt New “contract test” CMake project that validates optional solver targets/definitions match expectations.
cmake/polysolve/polysolve_optional_dependency.cmake New helper module to find system deps, decide whether to fetch, and run compile/link probes.
cmake/recipes/accelerate.cmake New recipe for Apple Accelerate with linkability probing and graceful disable behavior.
cmake/recipes/mkl.cmake Adds internal MKL target + probe gating, ARM/platform guards, and more graceful failure paths.
cmake/recipes/onetbb.cmake Adds system-probe-first behavior and fetch gating for oneTBB (required for MKL TBB threading).
cmake/recipes/blas.cmake Switches BLAS discovery to QUIET mode and makes BLAS optional (warn instead of hard fail).
cmake/recipes/lapack.cmake Switches LAPACK discovery to QUIET mode and makes LAPACK optional (warn instead of hard fail).
cmake/recipes/suitesparse.cmake Adds system probe logic and makes SuiteSparse failures non-fatal, disabling solvers instead.
cmake/recipes/superlu.cmake Makes SuperLU discovery quiet, adds linkability probe, and only exposes alias after success.
cmake/recipes/spqr.cmake Fixes target name check and adds system probing for SuiteSparse/SPQR.
cmake/recipes/pardiso.cmake Makes Pardiso discovery quiet, adds LAPACK check + linkability probe, and only exposes alias after success.
cmake/recipes/hypre.cmake Adds system-probe-first behavior and fetch gating (disable if missing and fetch disabled).
cmake/recipes/amgcl.cmake Adds system-probe-first behavior and fetch gating (disable if missing and fetch disabled).
cmake/recipes/spectra.cmake Adds system-probe-first behavior and fetch gating (disable if missing and fetch disabled).
cmake/recipes/boost.cmake Adds system probing (including Boost::headers aliasing) before fetching.
cmake/recipes/eigen.cmake Adds system probe path and consolidates Eigen target configuration in a helper function.
cmake/recipes/spdlog.cmake Adds system probing before fetching; keeps spdlog required and hard-fails if unavailable.
cmake/recipes/json.cmake Adds system probing before fetching; keeps JSON required and hard-fails if unavailable.
cmake/recipes/LBFGSpp.cmake Fixes target naming and adds system probing before fetching; keeps LBFGSpp required.
cmake/find/FindSuperLU.cmake Switches compile checks to C++ source compiles for version probing.
cmake/find/FindPardiso.cmake Adjusts package handle naming to Pardiso (aligning found-variable naming).
.github/workflows/optional-dependencies.yml New CI workflow to validate optional dependency contract scenarios across OSes.
.github/workflows/coverage.yml Simplifies the coverage matrix/caching by removing the unused threading dimension.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +291 to +298
set(check_result_var "POLYSOLVE_${check_id}_LINKABLE")
unset(${check_result_var} CACHE)
try_compile(${check_result_var}
"${check_binary_dir}"
"${check_source}"
CMAKE_FLAGS ${check_cmake_flags}
COMPILE_DEFINITIONS ${POLYSOLVE_CHECK_COMPILE_DEFINITIONS}
LINK_LIBRARIES ${POLYSOLVE_CHECK_TARGET} ${POLYSOLVE_CHECK_LINK_LIBRARIES}
Comment thread cmake/recipes/mkl.cmake
Comment on lines +178 to +179
message(WARNING "Could not find ${var}.")
return()
Comment thread tests/CMakeLists.txt

if(POLYSOLVE_WITH_AMGCL)
target_compile_definitions(unit_tests PRIVATE -DPOLYSOLVE_WITH_AMGCL)
target_compile_definitions(polysolve_unit_tests PRIVATE -DPOLYSOLVE_WITH_AMGCL)
Comment on lines 21 to +22
target_compile_definitions(SuperLU_SuperLU INTERFACE ${SUPERLU_DEFINES})

-DCMAKE_DISABLE_FIND_PACKAGE_LAPACK=ON
-DCMAKE_DISABLE_FIND_PACKAGE_SUPERLU=ON
-DCMAKE_DISABLE_FIND_PACKAGE_Pardiso=ON
-DCMAKE_DISABLE_FIND_PACKAGE_SPQR=ON
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants