Skip to content

Add cqr_mkl_?syrk_compact: batched symmetric rank-k update - #30

Open
ivan-pi wants to merge 4 commits into
mainfrom
claude/syrk-routines-review-qhi0oe
Open

Add cqr_mkl_?syrk_compact: batched symmetric rank-k update#30
ivan-pi wants to merge 4 commits into
mainfrom
claude/syrk-routines-review-qhi0oe

Conversation

@ivan-pi

@ivan-pi ivan-pi commented Aug 6, 2026

Copy link
Copy Markdown
Owner

Summary

Implement cqr_mkl_?syrk_compact and dsyrk_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 missing mkl_?syrk_compact that 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:

    • Precisions: double and single
    • Layouts: column-major (tuned) and row-major
    • Transpose modes: A*A^T and A^T*A
    • Triangle selection: upper and lower
    • Interleave widths: V ∈ {2, 4, 8, 16}

    The 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 using BatchView abstractions.

  • Dispatch layer (src/cqr_syrk_compact_dispatch.cpp): LAPACK/BLAS-style argument validation with info = -j error reporting; runtime dispatch on interleave width V to compile-time instantiations.

  • MKL adapter (src/cqr_mkl_syrk.cpp): C-linkage wrapper cqr_mkl_?syrk_compact that unwraps MKL enums and MKL_COMPACT_PACK format 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_compact entry 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:
      • Suite A: vs. dense per-matrix cblas_?syrk (whole matrix, gates active triangle + opposite untouched)
      • Suite B: vs. mkl_?gemm_compact (triangle comparison, same compact pipeline)
      • Suite C: end-to-end Cholesky QR (G = A^T A → R = chol(G) → Q = A R^{-1}; verify Q R = A and Q^T Q = I)
  • 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

claude added 4 commits August 7, 2026 06:15
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
ivan-pi force-pushed the claude/syrk-routines-review-qhi0oe branch from 57538c4 to 4f76f90 Compare August 7, 2026 06:18
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