Repository navigation
Tests: validate against LAPACKE/CBLAS instead of hand-rolled references - #61
Merged
Merged
Conversation
The hand-rolled scalar references of the portable suites (ref_geqr2, ref_orm2r, ref_org2r, ref_geqp3, ref_potf2, ref_trsm, ref_gels, matmul, tri_apply) now forward to LAPACKE_?geqr2 / ?ormqr / ?orgqr / ?geqp3 / ?potrf / ?gels and cblas_?trsm / ?trmm / ?gemm through the new tests/test_lapack_util.hpp, which hands a MatrixView's data and leading dimension to the library and stages the few LAPACKE operands that have no transpose flag. The unpivoted LDL^T keeps its scalar reference (LAPACK has none) and validates it against cblas_?trmm first. Where the reference is LAPACK's blocked driver the gates are relative to the operand norms at a multiple of n eps (the design documents' 7.1 gates); only ?geqr2 stays an elementwise ~eps comparison. cmake/FindLAPACKE.cmake finds the stack (MKL's own with the MKL extension, otherwise OpenBLAS, Netlib, or any lapacke + find_package(LAPACK) pair; -DCBK_TEST_LAPACK pins one, which matters under Debian's alternatives symlinks) and exposes it as LAPACKE::LAPACKE. The dense-LAPACK invariants of the geqrf and potrf MKL suites moved into the portable suites, so they run on every stack; the MKL suites keep the compat API coverage and the cross-checks against MKL's compact kernels. CI runs OpenBLAS and Netlib under gcc and clang next to the MKL jobs; a tag-only macOS workflow targets Accelerate through accelerate-lapacke (unverified). Closes #27. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_01GdNaquWMN3b39gvTsXZyjn
.github/workflows/macos.yml runs the suites on macos-latest on the same triggers as the Linux CI: it builds accelerate-lapacke (Netlib's LAPACKE over Accelerate's LAPACK; a Fortran compiler from Homebrew's gcc for its configure) into ~/.local and configures cbk with -DCBK_TEST_LAPACK=accelerate. FindLAPACKE.cmake's Accelerate branch looks up the installed `lapacke` package under its own lower-case config name (so the module is not re-entered), takes CBLAS from Accelerate's vecLib sub-framework through the SDK's framework directory (xcrun --show-sdk-path), and never includes the <Accelerate/Accelerate.h> umbrella, whose LAPACK prototypes are the same C symbols lapacke.h declares with different parameter types. The test util spells the CBLAS layout enum CBLAS_ORDER, the one name every CBLAS header defines. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_01GdNaquWMN3b39gvTsXZyjn
PLANS.md was reduced on main to status and open items; the issue #27 entry is re-applied on that version. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_01GdNaquWMN3b39gvTsXZyjn
accelerate-lapack refuses to configure without CMAKE_OSX_SYSROOT (it matches the SDK's Accelerate stubs to the system), and a workflow preset takes no cache variables, so the build is a configure and a build call. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_01GdNaquWMN3b39gvTsXZyjn
AppleClang's default standard is older than gcc's and clang's on Linux, and the library's cxx_std_17 is a PRIVATE build requirement of the kernels that nothing propagates, so every test, example and benchmark target now requests the feature itself (reproduced on Linux with -DCMAKE_CXX_STANDARD=14; green with the change). The macOS workflow installs accelerate-lapacke, pinned to v2.0.0, into its own prefix and caches that under a key naming the release and the macOS SDK it was linked against; the Fortran compiler is installed only on a cache miss. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_01GdNaquWMN3b39gvTsXZyjn
… cleanups cmake/FindLAPACKE.cmake shrinks to what FindLAPACK does not know: with the MKL extension the stack is MKL::Compact's; otherwise find_package(LAPACK) picks the implementation under BLA_VENDOR (OpenBLAS, Generic, Apple) and the module adds lapacke.h, cblas.h (a distro's OpenBLAS subdirectory included) and liblapacke where LAPACKE is a library of its own, then runs one link test. An unset BLA_VENDOR means Generic, not FindLAPACK's "All": that took an installed MKL first and, under Netlib's liblapacke and OpenBLAS's cblas.h, put three LAPACK stacks in one binary (the suites failed that way). The CBK_TEST_LAPACK option goes; CI passes BLA_VENDOR from a single stack axis whose rows carry their apt packages, and the macOS job passes Apple. CMAKE_CXX_STANDARD 17 (no extensions) is set once for every target instead of a cxx_std_17 line per executable. From the review of the diff: ref_larfg and its dispatch entry were dead; lapack_ld is MatrixView::ld(); Staged stages only writable operands (ormqr's and gels's right-hand sides), so the const machinery goes; ref_gels factors A in place and runs ?gels on the copy; ACCELERATE_NEW_LAPACK lives on the target only; gen_matrix builds on gen_boosted; the geqrf invariants skip the ungated ?geqrf comparison on structured inputs and run per precision through one template; the gels and potrf suites keep one tolerance where three names held one value; sytrfnp's reference check runs one triangle through the transposed view. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_01GdNaquWMN3b39gvTsXZyjn
actions/cache rejects a path containing '..' ("Relative pathing '.' and
'..' is not allowed") and saved nothing, so every run rebuilt
accelerate-lapacke. The install prefix is now under the runner's temp
directory, an absolute path, set at the job level where that context is
available.
Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GdNaquWMN3b39gvTsXZyjn
A job-level env cannot read runner.temp, and the workflow file failed to parse (a run with no jobs). The cache path names the runner's temp directory in the step itself and the shell steps use $RUNNER_TEMP. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_01GdNaquWMN3b39gvTsXZyjn
ref_ormqr, ref_orgqr and ref_potrf forward to LAPACKE_?ormqr, ?orgqr and ?potrf, the blocked drivers, so the unblocked names orm2r, org2r and potf2 no longer described them. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_01GdNaquWMN3b39gvTsXZyjn
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
Refactor the test suites to validate against real LAPACKE and CBLAS libraries instead of hand-rolled scalar reference implementations. This addresses issue #27 and strengthens the correctness gates by comparing against standard dense LAPACK implementations across multiple vendor stacks (MKL, OpenBLAS, Netlib, Accelerate).
Key Changes
New
test_lapack_util.hpp: Header-only utility providinglapack<T>struct with static forwarders to LAPACKE and CBLAS entry points, plus helper functions to mapMatrixViewoperands to library arguments (layout, leading dimension). Supports MKL (<mkl_lapacke.h>,<mkl_cblas.h>), Accelerate (<lapacke.h>,<vecLib/cblas.h>), and standard (<lapacke.h>,<cblas.h>) stacks via compile definitions.New
cmake/FindLAPACKE.cmake: CMake module to locate and link a LAPACKE + CBLAS stack. Supports MKL (viaFindMKLCompact.cmake), OpenBLAS (pkg-config or manual), Netlib (with distro path suffixes), and Accelerate (macOS). Validates each candidate with a link test callingLAPACKE_dgeqrf,LAPACKE_dgeqr2,cblas_dgemm, andcblas_dtrsm. ExposesLAPACKE::LAPACKEtarget andLAPACKE_VENDOR_FOUNDvariable.New
.github/workflows/macos.yml: CI job for macOS runners building accelerate-lapacke (Netlib's LAPACKE over Accelerate's LAPACK 3.9.1) and running tests against it.Refactored
test_compact_util.hpp: Removed hand-rolled scalar reference kernels (ref_larfg,ref_geqr2,ref_orm2r,ref_trsm_upper,ref_potf2,ref_gels,matmul,tri_apply,solve_errors). These now live intest_lapack_util.hppas wrappers around LAPACKE/CBLAS. Updated documentation to clarify the split: shared helpers remain here, dense references moved to the library-backed utility.Updated portable test suites (
test_geqrf_compact.cpp,test_potrf_compact.cpp,test_ormqr_compact.cpp,test_orgqr_compact.cpp,test_gels_compact.cpp,test_trsm_compact.cpp,test_sytrfnp_compact.cpp): Changed to includetest_lapack_util.hppinstead oftest_compact_util.hppand now validate against LAPACKE/CBLAS. Tolerances and gate descriptions updated to reflect comparison against library implementations (e.g., "LAPACKE_?geqr2" instead of "scalar geqr2").Updated MKL test suites (
test_geqrf_mkl.cpp,test_potrf_mkl.cpp, etc.): Removed duplicate dense-LAPACK invariant suites (now in portable tests). MKL suites focus on MKL-specific cross-checks and the Compact pipeline. Updated comments to clarify the split of responsibilities.Updated
test_mkl_util.hpp: Clarified thatlapack<T>now comes fromtest_lapack_util.hpp(included via the MKL utility). Removed "MKL-free" language; now "shared helpers."CMakeLists.txt: Added
CBK_TEST_LAPACKcache variable (auto | mkl | openblas | netlib | accelerate) to select the LAPACKE stack. Defaultautouses MKL with the extension, otherwise tries OpenBLAS then Netlib. Integrated `FindLAPACKEhttps://claude.ai/code/session_01GdNaquWMN3b39gvTsXZyjn