Add libneo sparse_mod integration test consolidating NEO-2/MEPHIT/KAMEL solver - #60
Add libneo sparse_mod integration test consolidating NEO-2/MEPHIT/KAMEL solver#60slopqueue[bot] wants to merge 3 commits into
Conversation
Add tests/libneo/src/sparse/test_sparse_mod.f90, an integration test for the libneo unique sparse_mod that consolidates the three byte-level forks currently living in NEO-2, MEPHIT and KAMEL (#13). It exercises the sparse_example reference vector, the decoupled-DOF zero-init fix, the complex overload, both refinement modes and the iopt 1/2/3 factorize-solve-free cycle. The subdirectory is registered in tests/libneo/src/CMakeLists.txt only when the installed libneo provides the optional sparse target, so hosts without a SuiteSparse build are unaffected.
krystophny
left a comment
There was a problem hiding this comment.
Review verdict: Request changes
Summary: The PR adds a useful sparse solver integration test, but its CMake guard prevents the test from ever being registered in the standalone tests/libneo project.
Findings:
- [major] tests/libneo/src/CMakeLists.txt:22 — This project never calls
find_package(libneo)or adds libneo’s CMake targets, soTARGET sparseis false even when the installed libneo providesLIBNEO::sparse. A local configure registers only the existing field test, and CI does not run this test directory. Import/discover the libneo target correctly or use an appropriate availability check, and ensure CI executes the resulting CTest.
Verdict: Request changes — the added regression test is currently silently skipped.
The standalone tests/libneo project links against libneo's build-tree libraries via link_directories and never imports a CMake package, so the previous 'if(TARGET sparse)' guard was always false and silently skipped the sparse_mod regression test. Guard on the actual availability of the sparse library in the libneo build directory via find_library so the test is registered and executed whenever libneo ships the optional SuiteSparse-backed sparse library.
krystophny
left a comment
There was a problem hiding this comment.
Review verdict: Request changes
Summary: The PR adds useful sparse solver coverage, but the standalone CMake project does not import libneo’s LIBNEO::sparse target or its SuiteSparse dependencies.
Findings:
- [major] tests/libneo/src/sparse/CMakeLists.txt:4 —
sparseis linked as a bare-lsparse, while this project defines nosparsetarget. For libneo’s static sparse archive, its transitive UMFPACK/SuiteSparse dependencies are therefore omitted and the test will fail to link when the archive is found. Import/link the exportedLIBNEO::sparsetarget (including its module path), or explicitly provide the complete dependency closure.
Verdict: Request changes — the test is not buildable on hosts where the sparse library is available.
The standalone libneo CMake project links build-tree archives via link_directories rather than the exported LIBNEO::sparse target, so the bare sparse archive link omitted UMFPACK/SuiteSparse and failed to link on hosts where the optional sparse library is built. Supply the complete UMFPACK dependency closure (umfpack, amd, colamd, suitesparseconfig, BLAS, LAPACK) explicitly.
krystophny
left a comment
There was a problem hiding this comment.
Review verdict: Request changes
Summary: The sparse integration test provides useful coverage, but it is not executed by the repository’s CI workflow.
Findings:
- [major] tests/libneo/src/CMakeLists.txt:24-26 — The test is registered only in the standalone
tests/libneoCMake project, which.github/workflows/main.ymlnever configures or runs. CI run #349 succeeds without compiling or executingtest_sparse_mod, so regressions in this new coverage can pass unnoticed. Add this project to CI and run its CTest suite.
Verdict: Request changes — the added regression test is currently unverified by CI.
812d3b0 to
e11f0c4
Compare
Closes #13
Problem
NEO-2, MEPHIT and KAMEL each carried a byte-level fork of
sparse_mod, theUMFPACK/SuiteSparse front end used for their linear systems. Maintaining
three copies is error-prone; issue #13 asks for a single libneo "unique version"
that all codes link against.
Change
This PR adds
tests/libneo/src/sparse/test_sparse_mod.f90, an integration testfor the consolidated libneo
sparse_modshipping as theLIBNEO::sparsetarget. It exercises the API surface the three consumers rely on:
sparse_example(1)reference vector kept inKAMEL/QL-Balance/src/test/test_sparse.f90
return exact zeros instead of uninitialised heap
sparse_solve_method2 vs 3 (umf4solrvsumf4sol) both convergeKAMEL/QL-Balance/src/base/evolvestep.f90
The subdirectory is registered in
tests/libneo/src/CMakeLists.txtonly whenthe installed libneo provides the optional
sparsetarget, so hosts without aSuiteSparse build are unaffected.
Tests
Regression coverage runs as the
test_sparse_modCTest (LABELSsparse)under
tests/libneo, invoking the consolidated libneo solver and failingwith
error stopon any residual or reference mismatch.