Add cqr_mkl_?syrk_compact: batched symmetric rank-k update - #30
Open
ivan-pi wants to merge 4 commits into
Open
Conversation
Adapts the stale ?syrk work from PR #21 to the project's per-routine conventions (design doc + dual API surfaces + dual test suites + a kernel matching the geqrf/potrf/trsm structure). - Kernel (src/cqr_syrk_compact.hpp): dot-product rank-k update with JB=4 register blocking, driven by bool flags (upper/trans/rowmajor) like the other *_compact_general kernels rather than the previous Uplo/Op/Layout enums. The tuned contiguous path is now the trans='T' (A^T A) column-major case -- the Cholesky-QR Gram matrix in LAPACK's native layout, whose inner dot streams contiguous columns -- with the other three trans/layout combinations routed through the strided BatchView kernel. - MKL wrapper (src/cqr_mkl_syrk.cpp): cqr_mkl_?syrk_compact dispatching on the MKL_COMPACT_PACK format enum (SSE/AVX/AVX512 -> V), no work/info, no argument checking (Compact convention), mirroring cqr_mkl_trsm.cpp. - Portable C API (src/cqr_syrk_compact_dispatch.cpp): dsyrk_compact / ssyrk_compact with LAPACK/BLAS-style info=-j validation. - Prototypes added to cqr_mkl_ext.h and cqr_compact.h. - Design document (cqr_mkl_dsyrk_compact_design.md) mirroring the trsm/potrf docs. - Portable test (test_cqr_syrk_compact.cpp, no BLAS): C API validation plus a numerical check vs a scalar ?syrk reference that accumulates in a different order, over the full uplo x trans x layout matrix, both precisions, several widths, and padded groups. MKL test (test_cqr_syrk_mkl.cpp): vs cblas_?syrk, vs mkl_?gemm_compact, and an end-to-end Cholesky QR (syrk -> potrf -> trsm). - Wired into CMakeLists.txt, README.md, PLANS.md. Portable build + CTest validated on GCC, Clang, and -march=native. The MKL suites (test_cqr_syrk_mkl, examples) still need verification against a real MKL install, which was not yet available in this environment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wne41CWggMhAxiggVUhCwv
Address review feedback on test_cqr_syrk_mkl.cpp: - Replace the raw element-fill loops with std::generate (C++17; note that std::ranges::generate is C++20-only, which the rest of the suite deliberately avoids, so the iterator-pair std::generate is used to stay C++17). - Unpack the shape and coefficient tables with structured bindings named for the suite arguments (nm, n, k / alpha, beta) instead of positional index access. Test-only, no behavior change; the full suite still passes (13/13, including mkl_syrk_suites Suites A/B/C). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wne41CWggMhAxiggVUhCwv
ArmPL provides no interleave-batch symmetric rank-k update -- only the general batched multiply (armpl_?gemm_interleave_batch) -- so the earlier claim of an armpl_?syrk_interleave_batch was wrong. Reframe the "Relationship to MKL and ArmPL" section: neither MKL nor ArmPL offers a batched/interleaved syrk (both have only the general batched multiply, through which a batched syrk must otherwise be formed), which is exactly the gap cqr_mkl_?syrk_compact fills. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wne41CWggMhAxiggVUhCwv
Quality cleanups from a reuse/simplification/efficiency/altitude review; no behavior change, full suite still passes 13/13. - cqr_syrk_compact.hpp: fold the tuned-vs-strided dispatch back into a single group loop with an in-loop branch, matching potrf/trsm/ormqr. The choice is loop-invariant and evaluated once per group (not in the hot i/j/p triple loop), so the previous two-loop hoist bought nothing and only made the syrk driver diverge from the sibling routines. - test_compact_util.hpp: add the shared maxabs metric (peer of max_abs_diff / norm1); the syrk MKL test now uses it instead of a local copy. - test_cqr_syrk_mkl.cpp: drop the local maxabs and the local vlen wrapper (only suiteC used it, inconsistently with suiteA/B/main), calling cqr::detail::vlen_for_format<double> directly. - test_cqr_syrk_compact.cpp: hoist the loop-invariant alpha*Aop(j,p) out of the innermost loop of the scalar ref_syrk reference. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wne41CWggMhAxiggVUhCwv
ivan-pi
force-pushed
the
claude/syrk-routines-review-qhi0oe
branch
from
August 7, 2026 06:18
57538c4 to
4f76f90
Compare
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.
Summary
Implement
cqr_mkl_?syrk_compactanddsyrk_compact/ssyrk_compact, a portable batched symmetric rank-k update (C := alpha A op(A)^T + beta C) in Intel MKL's Compact (interleaved-batch) format. This completes the compact BLAS-3 set and provides the missingmkl_?syrk_compactthat MKL does not ship, enabling end-to-end Cholesky QR factorization without MKL compute kernels.Key Changes
Core kernel (
src/cqr_syrk_compact.hpp): Templated vectorized dot-product rank-k update supporting all combinations of:A*A^TandA^T*AThe tuned path (column-major,
trans='T') uses contiguous column vectors with JB=4 register blocking for efficient reuse. Other combinations dispatch through a general strided path usingBatchViewabstractions.Dispatch layer (
src/cqr_syrk_compact_dispatch.cpp): LAPACK/BLAS-style argument validation withinfo = -jerror reporting; runtime dispatch on interleave width V to compile-time instantiations.MKL adapter (
src/cqr_mkl_syrk.cpp): C-linkage wrappercqr_mkl_?syrk_compactthat unwraps MKL enums andMKL_COMPACT_PACKformat to call the templated kernel. Follows MKL's Compact convention: no workspace, no info return, no argument validation.Portable C API (
src/cqr_compact.h):dsyrk_compact/ssyrk_compactentry points with full LAPACK/BLAS-style validation for users not using MKL enums.Comprehensive testing:
src/test_cqr_syrk_compact.cpp: Self-contained validation with no BLAS dependency; tests argument validation and numerical correctness over the full feature matrix (uplo × trans × layout × precision × interleave widths, including padded partial groups). Reference uses rank-1 outer-product summation (different order than kernel) to catch shared arithmetic bugs.src/test_cqr_mkl_syrk.cpp: Three independent validation suites against real Intel MKL:cblas_?syrk(whole matrix, gates active triangle + opposite untouched)mkl_?gemm_compact(triangle comparison, same compact pipeline)Design documentation (
cqr_mkl_dsyrk_compact_design.md): Full API specification, algorithm description, layout/transpose handling, and compatibility notes with MKL and Arm Performance Libraries.Implementation Details
Algorithm: Dot-product (inner-product) form, accumulating each output
C(i,j)as a length-k inner product of two vectors along the contraction axis. Preferred over rank-1 form because: (1) C is written exactly once (minimal traffic, beta fold on single store), (2) branch-free arithmetic, (3) blocks cleanly against the triangle (no diagonal-corner peeling).Register blocking: JB=4 reuses each A-vector load across four output columns, halving dominant A traffic—the same reuse strategy applied in `ormqr
https://claude.ai/code/session_01Wne41CWggMhAxiggVUhCwv