[dmaldona]

Closes #118

Task: T31 — oneAPI and Metal extensions (split on main by #147: the allocation-free :ka fallbacks and the :auto order of #112 are now T32, #146). Merge #147 first.

What was built: ext/SparseDirectSolverOneAPIExt.jl (oneMKL sparse adapters, the T13 API, oneMKL BLAS and LAPACK bindings through the C shims, max_local_bytes from the device, _auto_vendor_dense keeping LAPACK on KA under :auto), and ext/SparseDirectSolverMetalExt.jl (Float32 CSR upload, 32 KiB local-memory cap, fused level solves, cached offset-aware MPS Cholesky for regime-C fronts ≥ 64 wide whose status is folded into info, used only under impl = :auto with vendor calls allowed). CI legs oneapi (runner headroom) and test-metal (hosted macOS, Float32). Base fix: under KA 0.10 the CPU backend no longer counts as a GPU. _tri_decode seeds its integer square root from Float32 (Metal cannot compile isqrt's Float64).

Tests: oneAPI (Max 1550, all four element types): 64,637 pass / 0 fail / 4 broken. Metal (M4 Max, Float32, before round 4): 23,399 pass / 0 fail / 1 broken. After the round-4 fixes, CPU + CUDA (2× H200), all four element types, 13 affected files incl. Aqua: 77,371 pass / 0 fail / 0 broken. Full suite, AMDGPU, oneAPI and Metal on this head: pending CI.

Deviations from PLAN.md / TASKS.md: :auto resolves GEMM/SYRK to MPS through :generic on Metal (to be measured against :ka in T32). The MPS route allocates a command buffer per wide front per refactorization (#137). Every oneMKL call synchronizes the host, so regime-C fronts on oneAPI do too (PLAN §2.4; #148, T32). Metal is Float32 only (#143), and LDLᵀ/LU and batches run in regime C only (31-buffer limit); explicit options asking for anything else raise NotSupportedError. Metal batches use a serial stats reduction (#138). Details are in the Report.

Follow-up issues opened: #137, #138, #142 (addressed here: Float32 seed, timed on CUDA), #143, #144, #145, #148; task issue #146 (T32).

Review round 4 (head e536eeb)

1. MPS status is now folded into info on the device (_mps_status_info_kernel! after ka_chol_check_info!), with a failing-pivot test in a 96-wide front (test_numeric_cholesky_c.jl). On Metal it asserts info falls into the failing front; elsewhere it must equal the reference.
2. Scope: the owner split the task (TASKS: split T31, the allocation-free :ka fallbacks become T32 #147, T32 — Allocation-free KA dense fallbacks, :auto prefers :ka (issues #112, #53) #146), so this PR closes T31 — oneAPI and Metal extensions #118 only.
3. The MPS route is gated on impl = :auto with vendor_c (_front_impls(...).mps), so impl = :ka and "algo1" stay on KA. A testset pins this.
4. Metal: explicit "algo1" or subtree_budgets for S/H/G or batches raise NotSupportedError; the backends.md text is fixed.
5. The Report matches the branch (compat after 698e447, AGENTS.md KA rule, follow-ups cited).
6. Nits: _tri_decode (H200 medians: isqrt 52.5 ms, binary search 71.4 ms, Float32 seed 49.7 ms on lap2d(300)); subtree_max_fronts = 32 on Metal only; METAL_OPTIMIZATION_PLAN.md moved to bench/metal_optimization.md; oneAPI wording in index.md/README; docs/Project.toml oneAPI 2.10; oneMKL potrf/getrf exercised under explicit :vendor on oneAPI; the queue-ordering comment on the MPS command buffer.

The round-4 Metal and oneAPI changes have only been checked in CI; this machine has no Metal or Intel GPU.

Review round 5 (head dd7be99)

The oneapi CI leg failed 80 assertions in test_dense (potrf 100/257, potrf_info!, every element type). Round 4 made the oneAPI loops also run explicit impl = :vendor. oneMKL 2026.1's GPU potrf names the failing column relative to its 32-column panel: column 50 comes back as 18, or as 1042 for complex types (#150, JuliaGPU/oneAPI.jl#755).

• The factor follows LAPACK potf2, so after a oneMKL failure the oneAPI extension's vendor_potrf!/vendor_potrf_info! read the failing column from the diagonal (_onemkl_potrf_column). The shim has already synchronized, so this adds no host synchronization.
• On the reviewer's nit, the fix stays in ext/. The first push (5e689a6) did the scan in the core status checks for every vendor. dd7be99 restores _ka_chol_check_kernel! and potrf! to round 4, so the cuSOLVER/rocSOLVER paths are unchanged.
• test_dense adds j = 33 to the failing-pivot cases on every backend.
• Upstream: oneMKL: return LAPACK info from potrf!/getrf!/sytrf! through LinearAlgebra.LAPACK JuliaGPU/oneAPI.jl#754 makes LinearAlgebra.LAPACK.potrf!/getrf! return info, which fixes cholesky/lu on oneArrays. The extension does not depend on it.

Tests on the runner headroom (Data Center GPU Max 1100), CPU + oneAPI, all four element types: full suite incl. Aqua on 5e689a6, 102,946 pass / 0 fail / 4 broken; test_dense, test_numeric_cholesky_c, test_api on dd7be99, 44,231 pass / 0 fail / 4 broken. Full suite on every backend: pending CI on this head.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MsLzHiNVTSTLU8Ebm3tgcC

[claude]

nit (non-blocking): subtree_max_fronts = 32 changes the regime mix of the "all three regimes" fixture on CPU, CUDA and AMDGPU too, and nothing says why Metal needed it (the override in the extension does not touch these explicit options). Add the reason in the comment above the constant; if it is a Metal-only need, condition it on the backend rather than changing the fixture for everyone.

[claude]

nit (non-blocking): a second top-level plan document is not part of the repository layout (AGENTS.md "Layout": PLAN.md TASKS.md RESEARCH.md AGENTS.md), and its "Execution Record" is exactly the material that belongs in the task's Report block in TASKS.md (measurements, deviations, follow-ups). Suggest moving this file under docs/src/man/ (as a Metal backend note) or bench/, and copying the Execution Record into the Report.

[claude]

nit (non-blocking): new weak dependency Metal (compat 1.11) needs its one-line justification in the Report (AGENTS.md "Session rules"), and a note on why 1.11 is the floor (the offset-aware MPSMatrix(buffer, desc, offset) constructor and the MPSMatrixDecompositionCholesky wrapper, presumably).

[claude]

Status buffer overrun / aliasing of aux. The MPS status buffer handed to MPSMatrixDecompositionCholesky is N.aux, an MtlVector{Float32} of length nb = 1 (4 bytes, storage.jl:235). MPS writes an MPSMatrixDecompositionStatus, an NSInteger enum (8 bytes on arm64; the prototype in bench/metal_mps_proto.jl:18 correctly allocates MtlArray{MPS.MPSMatrixDecompositionStatus, 0} for it). Every wide front therefore writes 8 bytes into a 4-byte buffer. Page-granular buffer allocation hides it today, but it is an out-of-bounds device write, it clobbers aux (documented in storage.jl:139 as the max |aᵢⱼ| statistic, copied to the host by host_numeric), and the status is never read anyway.

Fix: give the plan its own status storage allocated once at allocate_numeric time — e.g. one MtlVector{MPS.MPSMatrixDecompositionStatus}(undef, 1) (or a UInt8 buffer of sizeof(MPSMatrixDecompositionStatus)) owned by MetalPotrfPlan (or one shared buffer stored in the plan and passed instead of aux), and drop the vendor_status/aux plumbing through _factor_front_c!/_factor_panel_c!. Keep the buffer referenced from the plan so the MTLBuffer stays alive.

[claude]

Allocation inside the numeric phase (must be recorded). This path creates an MPSCommandBuffer wrapper per wide front per (re)factorization; METAL_OPTIMIZATION_PLAN.md measures about 2.1 MB of host allocation per refactorization. AGENTS.md forbids allocation inside the numeric phase, and T31's own owner note (#53) is specifically about making the Metal/oneAPI dense path allocation-free per call. It is acceptable as a documented, measured exception only if the Report lists it as a deviation and a follow-up issue exists (the PR says "Follow-up issues opened: none"). Please open the follow-up (reusable command-buffer encoding) and cite it in the Report, or encode all wide fronts of a level into one command buffer created per launch to cut the count.

Also worth a comment here: ordering with the KA launches relies on Metal.jl's KA kernels and this command buffer sharing global_queue(device()); say so, since a future change to a private queue would silently break the dependency on the preceding assembly kernels and the following ka_chol_check_info!/trsm.

[claude]

Deviation from the task text, needs to be in the Report. T31 says: "on oneAPI/Metal :auto must prefer :ka" over the allocating :generic path. With this hook select_impl(:gemm/:syrk, …, :auto) on Metal resolves to :generic (mul! on MtlArray views → MPS with per-call descriptor allocation). If the measurement justifies it, fine, but it is the opposite of what the task asks and must be written into the Report's "Deviations" with the measured difference (the plan doc has numbers), and the comment here should say that :generic allocates per call on Metal.

nit (non-blocking): the sentence "POTRF/TRSM stay on KA" is no longer accurate for POTRF since _offset_potrf_plan routes fronts ≥ 64 wide to MPS; update the comment.

[claude]

nit (non-blocking): please say why the 256-work-item _cholesky_stats_kernel! reduction is replaced by a serial kernel plus a host reduction only when nbatch > 1 on Metal. If the shared reduction produced wrong info[ns + 1] for batches on Metal, that may be a latent bug of the kernel (e.g. a missing barrier that CUDA/AMD warps hide) and deserves a found-by-agent issue rather than a backend workaround; if it was a compiler limit, name it. As written, a reader cannot tell whether the two hooks are both needed (the serial kernel already writes info[ns + 1], and _numeric_info! recomputes it on the host).

[claude]

nit (non-blocking, but please measure on CUDA): _tri_decode sits in the innermost update loops of the regime-A and regime-B kernels on every backend (subtree.jl:181, front.jl:72, fused.jl:833/929/951). Replacing isqrt (one hardware sqrt) with a 16–32-iteration binary search that does an integer division per iteration adds a few hundred instructions per work item per tile on CUDA and AMDGPU. The two ±1 fixups that follow already make an approximate root exact, so a cheaper Metal-safe seed is unsafe_trunc(typeof(p), sqrt(Float32(8p + 1))) (exact enough for the p these kernels see: r is bounded by the 64 KiB local-memory classes, so 8p + 1 ≪ 2^24). Either way, the CUDA self-hosted job should confirm the regime-A/B timings in bench/ did not regress; the PR description does not mention this change at all.

[claude]

Weakened test. This round trip used to check that the vendor sparse constructors on CUDA and AMDGPU preserve ComplexF64 (issue #30 territory). Switching every backend to Float32 drops the only complex device transfer check in the helpers. Make the element type per backend instead:

        TC = backend_name(backend) == "Metal" ? Float32 : ComplexF64
        A = sprand(TC, 9, 9, 0.3) + I
        @test to_host(to_device(backend, A)) == A
        for INT in INTTYPES
            dA = to_device(backend, A, INT)
            @test to_host(dA) == A
            @test eltype(dA) == TC

The Float32 fallback for Metal is right since its to_backend rejects anything but Float32.

[claude]

VERDICT: CHANGES_REQUESTED (head 5575018)

Checked: the T31 section of TASKS.md on main, issue #118, AGENTS.md, the full diff of the four commits (Project.toml, the new Metal extension, src/dense/{capabilities,interface}.jl, src/numeric/{factorize,storage}.jl, src/solve/sweeps.jl, src/solver.jl, src/symbolic/schedule.jl, tests, docs, bench, METAL_OPTIMIZATION_PLAN.md), the surrounding code the new hooks plug into (aux allocation, ka_chol_check_info!, the pivot_epsilon semantics of the post-potrf check, front_ptr = layout.panel_ptr and the pivot handling of the fused solve kernel for regime-C fronts, _cholesky_stats_kernel! vs the serial variant, info_host sizing), git diff --check (clean), and gh pr checks (all CI legs still pending at review time; fix is the implementer workflow job, not a check). PLAN.md and Manifest.toml are untouched; public names and parameter strings are unchanged. I could not run the suite in this session (no permission to launch Julia); CI covers the CPU, CUDA and AMDGPU legs.

The Metal path itself reads correctly where I could verify it statically: MPS row-major plus lower = false yields the column-major lower factor, the post-MPS ka_chol_check_info! matches the :vendor/:generic pivot-epsilon semantics (#127), routing regime-C fronts through the fused level solve is layout-compatible (front_ptr is the panel pointer and the kernel applies piv), the serial stats kernel reproduces the parallel one field for field, _kernel_isqrt is exact, and the test-helper changes keep CPU/CUDA/AMDGPU schedules identical (_backend_schedule_options is the identity there).

Blocking findings

1. No Report block, no status marker, and the PR closes a task issue it does not complete. TASKS.md is not touched: T31 is still [ ], there is no Report (test counts, measurements, deviations, the one-line justification for the new weak dependency Metal, follow-ups). The body says Closes #118, but the T31 deliverables (1) oneAPI extension and (3) allocation-free :ka fallbacks with select_impl preferring :ka (#53, #112) are not delivered, so merging would close the task issue with two thirds of the task open. AGENTS.md requires an unfinished task to stay a draft with needs-owner, or the owner to split the task in TASKS.md first (e.g. a T31a: Metal extension section with its own Report); the PR is also ahead of T24 to T30 in the chain, which is for the owner to record. Either way the PR needs a Report block in TASKS.md whose claims match the diff before it can merge.
2. MPS status buffer overrun (ext/SparseDirectSolverMetalExt.jl:48-54): the status output of MPSMatrixDecompositionCholesky is an NSInteger enum (8 bytes); the buffer passed is N.aux, a MtlVector{Float32} of length 1 (4 bytes), so every wide front writes past the end and clobbers the aux statistic. Allocate a dedicated status buffer in the plan at allocate_numeric time (the prototype in bench/metal_mps_proto.jl already does it right).
3. Weakened test (test/test_helpers.jl:28-33): the complex (ComplexF64) device round trip on CUDA/AMDGPU was replaced by Float32 for every backend. Pick the type per backend so the vendor backends keep the complex check.

Must be recorded in the Report as deviations (not blocking by themselves)

• :auto on Metal resolves gemm/syrk to the allocating :generic path, the opposite of the task text (":auto must prefer :ka"); give the measurement that justifies it.
• Per-front MPSCommandBuffer creation inside refactorization (about 2.1 MB of host allocation per refactorization per the plan document) violates the "no allocation inside the numeric phase" rule; needs a follow-up issue and a Report entry ("Follow-up issues opened: none" is not accurate).
• ComplexF32 is excluded although the task says "Float32/ComplexF32 only"; LDLT/LU on Metal are forced to regime C (31-buffer limit); the SPD regime-C thresholds are overridden on Metal. All three are reasonable, all three belong in the Report.
• _tri_decode now uses a division-based binary search in the innermost loops of the regime-A/B kernels on every backend; confirm on the CUDA runner that regime-A/B timings did not regress, or seed from sqrt(Float32(...)) with the existing fixups (inline nit).

Nits (non-blocking): comment why the batched Cholesky stats reduction is bypassed on Metal (latent kernel bug or compiler limit?); explain the subtree_max_fronts = 32 change to the shared NUMERIC_ABC_OPTS fixture; move METAL_OPTIMIZATION_PLAN.md out of the repository root (its Execution Record is the material for the Report); update the "POTRF/TRSM stay on KA" comment now that MPS handles POTRF for fronts at least 64 wide.

[github-actions]

CI-FIX round 1 of 3: https://github.com/exanauts/SparseDirectSolver.jl/actions/runs/38506535424 failed on 5575018; Claude is diagnosing it.

[michel2323]

Owner-side review fixes (f0e3179) and open decisions

Fixed in f0e3179

• MPS status buffer (ext/SparseDirectSolverMetalExt.jl): the Cholesky status was written into N.aux, which overwrote the max |aᵢⱼ| statistic. The extension now allocates one MPSMatrixDecompositionStatus buffer per numeric allocation (new hook _offset_potrf_status), and the vendor_status argument of _factor_panel_c! is gone. Correction to the review: Metal.jl binds the status as Int32 (@cenum MPSMatrixDecompositionStatus::Int32), so there was no 8-byte overrun; the aliasing of aux was the real bug.
• Weakened helper test (test/test_helpers.jl): the ComplexF64 device round trip is back on CUDA and AMDGPU; only Metal uses Float32.
• AMDGPU CI failure (missed by the review, test_numeric_cholesky_a.jl:105, all element types): the PR made the test helpers pass max_local_bytes(backend) to the analysis, which rejects an explicit budget above it (resolve_subtree_budgets). The test decided whether to expect that error from subtree_local_bytes(budget), which rounds 1 MiB down to the 64 KiB class, so it expected success on ROCm. It now compares the raw budget.
• Report: T31 is [~] in TASKS.md with a Report covering the Metal part: build summary, M4 Max timings, the deviations the review listed, and follow-ups.

Tests on the owner's machine (affected files, all four element types): CPU 5,817 pass / 0 fail, CUDA (RTX 4080) 5,584 pass / 0 fail. AMDGPU: this CI run.

Still to be decided by the owner

1. Scope of T31 / T31 — oneAPI and Metal extensions #118. This PR delivers part (2) only (Metal). Part (1), the oneAPI extension, and part (3), allocation-free :ka fallbacks with :auto preferring :ka (Dense :ka/:generic paths and the KA CPU backend allocate in the numeric phase (Julia 1.10, coverage) #53, T23 remainder: oneAPI and Metal extensions, allocation-free KA dense fallbacks (#53) #112), are not done. Options: split T31 in TASKS.md (e.g. T31a: Metal extension, with oneAPI and the :ka work as separate tasks) and change Closes #118 accordingly, or keep the PR open. Merging as is closes T31 — oneAPI and Metal extensions #118 with two thirds of the task open.
2. Chain order. T31 lands ahead of T24–T30; record it in TASKS.md if that is intended.
3. :auto → :generic for GEMM/SYRK on Metal. This is the opposite of the task text, and no measurement against :ka is recorded. Accept as a deviation, or require the measurement first.
4. Allocation in the numeric phase. The MPS potrf route creates an MPSCommandBuffer per wide front per refactorization (about 2.1 MB per refactorization on lap2d_300). Accept as a documented exception with a follow-up issue, or require a fix before merge.
5. Follow-up issues to open (cited in the Report, not yet opened):
   • reusable MPS command-buffer encoding (item 4);
   • why Metal batches need the serial Cholesky statistics kernel and host info reduction (a latent bug in the 256-item reduction, or a Metal compiler limit?);
   • CUDA timing check of _tri_decode (binary search replacing isqrt in the regime-A/B inner loops on every backend);
   • ComplexF32 on Metal.

Not addressed (review nits)

• subtree_max_fronts = 32 in the shared NUMERIC_ABC_OPTS fixture changes the regime mix on every backend without a stated reason.
• METAL_OPTIMIZATION_PLAN.md sits in the repository root; its Execution Record is now summarised in the Report.

Pipeline note

Both automatic rounds on 5575018 (review fix, CI-FIX round 1) ended green without a push or a comment: the CI-fix runner has no GPU, and the only red job was AMDGPU, the same silent no-op as CI-FIX round 2 on #85. A post-step that labels needs-owner when a fix round leaves the head unchanged would catch this.

🤖 Generated with Claude Code

[claude]

Blocking (correctness of info on wide fronts): the MPS decomposition status is discarded, so a failed pivot inside a front ≥ 64 wide is only reported if MPS happens to leave a non-finite or non-positive entry on the diagonal.

The :vendor route (_potrf_info_impl!) writes cuSOLVER's devInfo into info[idx] and then runs ka_chol_check_info!; this route zeroes info[idx], runs MPS, and relies on the diagonal scan alone. What MPS writes into the result matrix on MPSMatrixDecompositionStatusNonPositiveDefinite is not documented as "NaN at the failing column" (LAPACK-style implementations stop at the failing column and leave the rest of the panel as the unfactored input, whose diagonal can be positive), so info may come back 0 with a garbage factor — the signal MadNLP uses for inertia regularization (#127).

Nothing in the suite covers this: the "first non-positive pivot" tests in test_numeric_cholesky_{b,c}.jl use singular_block_matrix(T, 60, 23) (banded, fronts far narrower than MPS_POTRF_MIN_WIDTH), and test_api.jl's info > 0 case is 40×40. The plan document's Phase 1 listed "factorization status behavior on an indefinite input" as a value to record; the Execution Record does not record it.

Fix, either of:

1. Fold the status into info: one MPSMatrixDecompositionStatus buffer per wide front (encode! takes a bare MTLBuffer, so the shared buffer cannot carry per-front values anyway), then a one-item KA kernel (or a copyto! into a view of info) that sets info[s] = w (or 1) when the status is nonzero, before ka_chol_check_info! refines it; or
2. Demonstrate on Metal that MPS leaves a detectable diagonal: add a test with an indefinite front at least 64 wide (e.g. random_spd(T, 300, …) with one root-separator diagonal entry negated, pivot_epsilon = 0), assert factorize! returns the reference info, and record the result in the Report.

Also nit (non-blocking): the comment above _offset_potrf_status ("never read") should then say why that is safe.

[claude]

nit (non-blocking), two inaccuracies in this paragraph:

• "Explicit schedule options are never replaced by this backend tuning" holds for the SPD/HPD thresholds only. For "S", "H", "G" and for every uniform batch, _backend_schedule_options(::MetalBackend, …) unconditionally sets factorization_alg to the vendor path and empties subtree_budgets, whatever the user passed. Say so (it is a hardware limit, which is fine, but a user who sets subtree_budgets explicitly should learn here why it has no effect).
• "offset-aware MPS decomposition objects cached at analysis" — they are built in allocate_numeric (numeric allocation), not in the analysis.

[claude]

nit (non-blocking): _kernel_isqrt is exact (invariant lo ≤ isqrt(x) < hi holds from the initial hi, mid ≥ 1 so the division is safe, termination is guaranteed), but it puts 16 (Int32) to 32 (Int64) integer divisions per decoded index into the innermost loops of the regime-A/B kernels on every backend. The Report defers the CUDA timing check to a follow-up; a cheaper exact variant would seed the two existing ±1 fixups from a Float32 square root (unsafe_trunc(T, sqrt(Float32(x)))), which Metal compiles and which is within ±1 of the true root for the front sizes regime A/B can hold (8p + 1 < 2^24). Either way the CUDA bench/ comparison needs to be recorded before the deviation is closed.

[claude]

nit (non-blocking): subtree_max_fronts = 32 changes the regime mix of the shared A+B+C fixture on CPU, CUDA and AMDGPU too, and neither the comment nor the Report says why (presumably so that Metal's 32 KiB cap still yields regime-A subtrees on laplacian2d(40,40)). Add the reason here in one line, and if the intent is Metal-only, apply it only when the backend's max_local_bytes is below 49152 so the other backends keep the fixture they had.

[claude]

VERDICT: CHANGES_REQUESTED (head f0e3179)

Checked: the T31 section of TASKS.md on main and the Report added on this branch, AGENTS.md, the full diff of the five commits (Project.toml, ext/SparseDirectSolverMetalExt.jl, src/dense/{capabilities,interface}.jl, src/numeric/{factorize,storage}.jl, src/solve/sweeps.jl, src/solver.jl, src/symbolic/schedule.jl, the test files and helpers, docs, bench, METAL_OPTIMIZATION_PLAN.md), the owner review-fix comment and the previous verdict, the code the new hooks plug into (_iview/_mview interleaving for the host info reduction, info sizing (ns+1)*nb on device and host, takes_c_path, SolvePlan built from the device Symbolic so _dense_solve_front sees MetalBackend, the width-independent loops of the fused solve kernels, _potrf_info_impl! for the :vendor/:generic status semantics, ka_chol_check_info!, vendor_c = alg != "algo3", the Options fields the Metal override reads), git diff --check (clean), gh pr checks (all legs pending at review time). PLAN.md and Manifest.toml are untouched; TASKS.md changes only in T31; public names, parameter and phase strings unchanged; the new weak dependency Metal is justified in the Report. I could not run Julia or fetch the Metal.jl sources in this session; CI covers CPU/CUDA/AMDGPU, the Metal numbers are the hardware run of the implementer.

What is right at this head: the two blocking code findings of the previous round are fixed (the MPS status has its own MPSMatrixDecompositionStatus buffer instead of aliasing N.aux; the ComplexF64 round trip is back on CUDA/AMDGPU). The Report block exists, T31 is [~], and every deviation the previous review asked for is recorded. The serial stats kernel reproduces the 256-item reduction field by field; the host info reduction uses the interleaved layout correctly; _kernel_isqrt is exact; the regime-C override for S/H/G on Metal is consistent with vendor_c; the AMDGPU test fix (maximum(opts.subtree_budgets) > max_local_bytes) is the right predicate given resolve_subtree_budgets rejects explicit budgets above the cap.

Blocking findings

1. MPS failure status is discarded (ext/SparseDirectSolverMetalExt.jl:63-71, inline comment). The :vendor route feeds the vendor devInfo into info before the diagonal check; the MPS route zeroes info[s], never reads the status, and relies on MPS leaving a non-finite or non-positive diagonal on a failed pivot, which is not documented behaviour. No test exercises a failing pivot in a front >= 64 wide (the info tests use the banded 60x60 singular_block_matrix and a 40x40 API case). Either fold the per-front status into info, or add a Metal test with an indefinite front >= 64 wide and record it in the Report. This is the info contract the MadNLP inertia correction depends on (#127).
2. Scope/closing is inconsistent: the Report marks T31 [~] with parts (1) oneAPI and (3) allocation-free :ka fallbacks undone, while the PR body says Closes #118 and the PR is not a draft. Green CI plus an approval would let the pipeline merge, close the task issue with two thirds of the task open, and chain the next task. Per AGENTS.md either the owner splits the task in TASKS.md on main (e.g. T31a — Metal extension, with this Report under it and Closes pointing at its issue), or the body drops Closes #118 and the PR stays open (draft, needs-owner). The owner listed this as an open decision; it has to be made before the verdict can be APPROVE.
3. Follow-ups without issues: the Report lists four follow-ups (reusable MPS command-buffer encoding for the per-front host allocation in the numeric phase; the serial stats kernel needed on Metal batches; the CUDA timing check of _tri_decode; ComplexF32 on Metal) and the PR body says "Follow-up issues opened: none". AGENTS.md requires found-by-agent issues cited in the Report for out-of-scope findings; the numeric-phase allocation in particular is an explicit rule exception that needs a tracked issue.

Non-blocking (inline nits): the docs paragraph in backends.md overstates "explicit options are never replaced" (S/H/G and batches are forced to regime C) and says the MPS objects are cached "at analysis" (numeric allocation); _kernel_isqrt puts 16-32 integer divisions per index into the regime-A/B inner loops on every backend (a Float32-seeded root with the existing fixups would be cheaper, and the CUDA bench check is still owed); subtree_max_fronts = 32 in the shared NUMERIC_ABC_OPTS fixture lacks a stated reason; METAL_OPTIMIZATION_PLAN.md still sits in the repository root.

[claude]

Blocking (carried over from the previous round, unchanged at this head): the MPS decomposition status is discarded. info[idx] is zeroed, the MPSMatrixDecompositionStatus the kernel writes into plan.status is never consulted, and the route relies on ka_chol_check_info! finding a non-finite or non-positive diagonal after a failed MPS Cholesky. Apple documents the panel contents as undefined on nonPositiveDefinite; if MPS leaves the (positive) input diagonal in place, the check passes and a wrong factor is reported with info = 0. This is the info contract the MadNLP inertia correction depends on (#127), and the :vendor route (vendor_potrf_info! → ka_chol_check_info!) does feed the vendor status in first.

Fix, either: (a) fold the status into info device-side, e.g. a one-item KA kernel that reads plan.status (it is a SharedStorage 0-d MtlArray, so a kernel can read it) and sets info[idx] = status == success ? 0 : 1 before ka_chol_check_info! runs; or (b) add a Metal test with an indefinite front at least MPS_POTRF_MIN_WIDTH wide (the existing info tests use the banded 60×60 singular_block_matrix and a 40×40 API case, so no wide front ever fails) proving that the diagonal check catches it, and record the reliance in the Report as a deviation.

[claude]

Blocking (Report does not match the diff): "KA compat stays "0.9, 0.10"" and (line 3335) "compat 2.9.2" are contradicted by the head commit 698e447 (Bump GPU packages): Project.toml now has KernelAbstractions = "0.10", oneAPI = "2.10", GPUArrays = "12", AMDGPU = "2.10", MadNLP = "0.10" and version = "0.2.0". Update these two sentences (and the AGENTS.md session-rules paragraph that still says the package admits KA 0.9.x and 0.10.x with the GPU stacks resolving 0.9) so the Report describes the branch that merges. Also note that docs/Project.toml keeps oneAPI = "2.9.2" while the package requires 2.10; the docs environment resolves only if oneAPI.jl main carries a 2.10.x version.

[claude]

Blocking (scope/closing, carried over): the Report leaves T31 [~] with part (3) (allocation-free :ka fallbacks, #53; :auto preferring :ka on backends without vendor bindings, #112) undone, while the PR body says Closes #118 and the PR is not a draft. Merging closes the task issue with a third of the task open and leaves #112/#53 dangling. Per AGENTS.md either (a) the owner splits the task on main first (e.g. T31 — oneAPI and Metal extensions done, T32 — allocation-free KA dense fallbacks (#53, #112) with the owner note moved there) and this Report and Closes line point at the split issue, or (b) the body drops Closes #118 and the PR stays open as a draft until part (3) lands. The same applies to the chain order (T31 lands ahead of T24–T30): record it in TASKS.md if intended.

[claude]

Report: cite the follow-up issues by number. #137 (MPS command buffer allocated per wide front per refactorization) and #138 (serial Cholesky stats reduction on Metal batches without a diagnosis) exist with label found-by-agent but the Report lists these follow-ups without their numbers, and the PR body still says "Follow-up issues opened: none". The remaining two follow-ups (the CUDA timing check of _tri_decode, now a 16–32-iteration division loop in the regime-A/B inner loops on every backend, and ComplexF32 on Metal) have no issue yet; AGENTS.md asks for a found-by-agent issue per out-of-scope finding cited in the Report.

[claude]

nit (non-blocking): this testset draws dense_hpd(T, 96) without Random.seed!(666) (test conventions: every numeric testset seeds). The assertions are structural, so nothing fails, but the final factorize! == 0 runs on an unseeded matrix. Same family of nit: the four backend_name(backend) == "Metal" && continue lines in test_fgmres.jl and test_refinement.jl could carry a one-word reason (Float64 fixtures).

[claude]

VERDICT: APPROVE (head e536eeb)

Checked: the T31 text on main and the owner split (commit 1ed3141 from owner/split-T31, PR #147: T31 = the two extensions, closes #118; T32 = #146), PLAN §1.3, §2.4, §2.6, §2.7, §3.9, AGENTS.md, the three previous verdicts and their inline findings, the full diff against main (40 files: Project.toml, ci.yml, AGENTS.md, both new extensions, the CUDA update! import, src/dense/{capabilities,interface}.jl, src/numeric/{factorize,storage}.jl, src/solve/sweeps.jl, src/solver.jl, src/symbolic/{device,schedule}.jl, the test helpers and sixteen test files, docs, README, bench), the code the hooks plug into (Options and its deep copy, resolve_subtree_budgets, _potrf_info_impl!/ka_chol_check_info!, the parallel stats kernel against the serial one, the _iview layout against the host info reduction, takes_c_path, the fused-pipeline callers of _factor_panel_c!/_front_impls, the ported cuDSS parameter sweep against the new Metal NotSupportedError), git diff --check (clean), and CI: this head is pending on every test leg (Aqua and Documentation green); on the previous head 2e2b976 the CPU, CUDA, AMDGPU and oneAPI legs passed, and the round-4 diff is confined to the MPS status fold, the :auto gating, the Metal schedule errors, _tri_decode, two testsets and docs. No Julia and no Metal/Intel hardware in this session.

Round-4 findings, all resolved: (1) the MPS route is an impl = :auto decision (_front_impls(...).mps = impl === :auto && vendor_c && _offset_potrf_enabled, tested in "offset-aware vendor potrf only under :auto": :ka and "algo1" leave vendor_ptrs empty and stay on ka_potrf!); (2) the MPSMatrixDecompositionStatus is folded into info on the device after ka_chol_check_info!, and "info: failing pivot in a wide front" puts the failing column inside a 96-wide regime-C front (Metal: info in the failing front; elsewhere equal to the reference); (3) explicit "algo1" or non-default, non-empty subtree_budgets for S/H/G or a batch on Metal raise NotSupportedError from the analysis path, the test helpers keep the replacing 3-argument method, and no test in the suite (including the ported cuDSS sweep, whose last accepted value is "algo2") trips the new error; (4) the Report matches the branch: compat after 698e447 (KA 0.10 only, oneAPI/AMDGPU 2.10, GPUArrays 12, MadNLP 0.10, version 0.2.0, AGENTS.md session rule updated), follow-ups cited by number (#137, #138, #142, #143, #144, #145, #148, task #146), the oneMKL host synchronization and the per-front command buffer listed as deviations, METAL_OPTIMIZATION_PLAN.md moved to bench/, subtree_max_fronts = 32 Metal-only, _tri_decode on a Float32 seed with the two exact fixups (timed on CUDA, 52.5 to 49.7 ms on lap2d_300).

Scope: PLAN.md and Manifest.toml untouched; the TASKS.md changes are the T31 section (marker [!], owner note, Report) plus the T32 section added by the owner; public names, parameter and phase strings unchanged; backend code in ext/, the core additions are generic hooks with CPU defaults; no @allowscalar; errors from src/errors.jl; docstrings on every new public method; the weak dependencies oneAPI and Metal are justified in the Report. The remaining rule exceptions (MPS command buffer allocation per wide front, oneMKL wait_and_throw per call, :auto to :generic for Metal GEMM/SYRK) are documented deviations with issues or T32.

Non-blocking (inline comments): the test-metal leg has never completed on this branch (one run cancelled at the 6-hour hosted limit, the others still running), so the round-4 Metal code has not executed on a Metal device yet; the Report should say so and the owner should run the affected files on the M4 Max before merging, and the job wants a timeout-minutes and a narrower pull-request selection. The new :auto testset lacks Random.seed!(666); the Metal continues in test_fgmres.jl/test_refinement.jl lack a stated reason.

Blocking findings: none.

[claude]

nit (non-blocking): this is a cross-backend change made for a oneMKL defect (#150). Letting the diagonal scan override a nonzero vendor info now also applies to cuSOLVER and rocSOLVER on the :vendor and batched :vendor routes (_potrf_info_impl!, potrf_batched! at interface.jl:433) and to potrf!, where a vendor that left something other than the failed pivot in A[j, j] would be reported at a different column than before. The reasoning (LAPACK potf2 leaves the failed pivot in place, and the T03 cuSOLVER finding shows it does the same) is sound and test_dense.jl pins info == j for j ∈ {1, 33, m÷2, m} on every backend, so this is fine once the CUDA and AMDGPU legs on this head are green; the Report says they have not run locally. A narrower alternative, if the owner prefers to keep the CUDA/AMDGPU vendor path untouched: do the diagonal scan inside the oneAPI extension's vendor_potrf!/vendor_potrf_info! (both already synchronize the host), so the oneMKL workaround stays in ext/.

[claude]

nit (non-blocking), please make the Metal status explicit: "covered on Metal by the test-metal CI leg only" reads as if that leg had run. It has not completed on any head of this branch: cancelled at the hosted 6-hour limit on 2698fae, 698e447 and e536eeb, in progress on this head. So nothing after the implementer's run has executed on a Metal device, and round 5 also touches the Metal path (_ka_chol_check_kernel! is what _offset_potrf_info! calls after the MPS decomposition). Suggested wording: "The round-4 and round-5 changes have not run on Metal: the test-metal leg has not completed on any head (cancelled at the 6-hour limit three times)." Giving that job a timeout-minutes below 360 and a narrower SDS_TEST_ONLY on pull requests would turn the cancellation into a signal; owner's call.

[claude]

VERDICT: APPROVE (head 5e689a6)

Checked: the T31 text on main and the owner split carried in the branch (commit 1ed3141 from PR #147: T31 = the two extensions, closes #118; T32 = #146), PLAN §1.3, §2.4, §2.6, §2.7, §3.9, AGENTS.md, the four previous verdicts and their inline findings, the full diff against main (41 files), and the round-5 commit 5e689a6 in detail (src/dense/fallback/potrf.jl, src/dense/interface.jl, the oneAPI extension header, test/test_dense.jl, docs/src/lib/oneapi.md, the Report). git diff --check is clean; PLAN.md and Manifest.toml are untouched; TASKS.md changes in one hunk (the T31 section with marker, owner note and Report, plus the T32 section the owner added); public names, parameter and phase strings are unchanged; both extensions live in ext/. No Julia and no GPU in this session.

Round 5 (the only change since the approved head e536eeb): the oneapi leg of e536eeb failed 80 test_dense assertions because oneMKL 2026.1 reports the failing potrf column relative to its 32-column panel (#150, found-by-agent). The fix makes the factor diagonal authoritative: _ka_chol_check_kernel! now scans unconditionally and overwrites info whenever it finds a bad diagonal entry (a reported failure it cannot see is kept), and potrf! returns the ka_chol_diag_info! column when nonzero. I checked the consequences: LAPACK potf2/potrf2 leave the failed pivot in A[j, j] and the already factored prefix has positive diagonals, so the first bad entry is the true column for the :generic path; the :ka path with the epsilon floor now reports the first column under the floor (documented in the Report as a side effect, consistent with #127); the MPS route on Metal is unchanged in effect (info is zeroed before the scan). test_dense.jl adds j = 33 to the failing-pivot cases on every backend and still runs the explicit :vendor oneMKL path through tested_impls. The Report records the headroom run (102,946 pass / 0 fail / 4 broken, CPU + oneAPI) and states honestly that CUDA and AMDGPU were not run locally.

CI: on this head every test leg is in progress (build green). On e536eeb the CPU, CUDA and AMDGPU legs passed, the oneAPI leg failed (fixed here), and test-metal was cancelled at the 6-hour limit, as on the two heads before it. Since the round-5 change also goes through the cuSOLVER and rocSOLVER :vendor routes, the owner should wait for green CUDA and AMDGPU legs before merging; the test-metal leg has never completed on this branch (see the TASKS.md nit).

Blocking findings: none.

Nits (non-blocking, inline): (1) src/dense/fallback/potrf.jl:136: the override of a nonzero vendor info is a cross-backend change for a oneMKL defect; acceptable with the tests that pin it, a narrower alternative would keep the scan inside the oneAPI extension. (2) TASKS.md:3384: say explicitly that rounds 4 and 5 have not run on a Metal device, since the test-metal leg has not completed on any head; a timeout-minutes below 360 with a narrower selection on pull requests would make it produce a signal.

[claude]

nit (non-blocking): the workaround now lives in the extension and keeps the cuSOLVER/rocSOLVER paths at round 4, which is what the previous nit asked for. One gap in its evidence: the Report says "the factor still follows LAPACK potf2 (the failed pivot stays in A[j, j])", but every failing-pivot case in test_dense.jl sets Sj[j, j] = -1 before the factorization, so the scan finds column j whether or not oneMKL writes the Schur-updated pivot back into A[j, j]. The case that distinguishes the two is a pivot that only turns non-positive during elimination (positive A[j, j], e.g. Sj[j, j] = S[j, j] - 1.5 * abs2(L[j, j]) with L = cholesky(S).L, so the original diagonal stays positive while the updated pivot is -0.5 abs2(L[j, j])). If oneMKL does not write back, st == 0 and the panel-relative value is returned, i.e. the pre-fix behaviour, but only on the explicit impl = :vendor path (:auto keeps potrf on KA here). Worth one such case on oneAPI in test_dense.jl to pin the potf2 claim; otherwise record it as an assumption in the Report.

[claude]

nit (non-blocking), carried over from the previous round: "covered on Metal by the test-metal CI leg only" still reads as if that leg had run. It has not completed on any head of this branch (cancelled at the hosted 6-hour limit on 2698fae, 698e447 and e536eeb; pending on dd7be99), so nothing after the implementer's M4 Max run has executed on a Metal device. Round 5 no longer touches the Metal path (the core status checks are back to round 4), so the statement is about round 4 only; please say so explicitly, e.g. "the round-4 changes have not run on Metal: test-metal has not completed on any head (cancelled at the 6-hour limit three times)". Two smaller points in the same spirit: the round-5 full-suite count on headroom (102,946 pass) was taken on the first push 5e689a6, and the final head dd7be99 ran test_dense, test_numeric_cholesky_c and test_api only (44,231 pass, as the PR body says); the Report should name the commit of each run. And the test-metal job has no timeout-minutes, so a hung Metal run costs 6 hours before it fails; a lower limit and a narrower SDS_TEST_ONLY on pull requests would turn the cancellation into a signal (owner's call).

[claude]

VERDICT: APPROVE (head dd7be99)

Checked: the T31 text on main and the owner split carried in the branch (PR #147, still open: T31 = the two extensions, closes #118; T32 = #146), PLAN §1.3, §2.4, §2.6, §2.7, §3.9, AGENTS.md, the five previous verdicts and their inline findings, the full diff against main (41 files) and the round-5 commits in detail: 5e689a6 (approved last round) and dd7be99, which moves the oneMKL potrf column workaround (#150) out of the core and into the oneAPI extension. git diff --check is clean; PLAN.md and Manifest.toml are untouched; TASKS.md changes in the T31 section (marker, owner note, Report) plus the T32 section the owner added; public names, parameter and phase strings unchanged; both extensions live in ext/; the new weak dependencies oneAPI and Metal are justified in the Report. No Julia and no GPU in this session.

Round 5, second push (dd7be99): src/dense/fallback/potrf.jl, src/dense/interface.jl, the Metal/CUDA/AMDGPU extensions and every test file except test_dense.jl are byte-identical to the approved round-4 head e536eeb, so ka_chol_check_info!, potrf! and the cuSOLVER/rocSOLVER :vendor routes are back to what the CUDA and AMDGPU legs passed on e536eeb. The oneAPI extension now does the scan itself: _onemkl_potrf_column returns oneMKL's info when it is 0 or -1 and otherwise the first diagonal entry that is not a finite positive real (ka_chol_diag_info!), keeping oneMKL's value when the scan finds none; vendor_potrf! and vendor_potrf_info! both go through it. The scan runs only on the failure path, after the shim has already waited for the queue, so it adds no synchronization to the numeric phase; uplo does not matter for a diagonal scan; the :vendor route in the numeric phase then keeps the nonzero info through ka_chol_check_info!. test_dense.jl adds j = 33 to the failing-pivot cases on every backend and runs explicit impl = :vendor on oneAPI through tested_impls. The docs sentence (lib/oneapi.md) and the Report follow the move.

Report: matches the diff (what moved where, the round-5 history, #150 and JuliaGPU/oneAPI.jl#754/#755, the headroom run counts, deviations and follow-ups #137, #138, #142, #143, #144, #145, #148, task #146). The PR body's "Merge #147 first" is for the owner: #147 is still open, and this branch already carries its TASKS.md edit.

CI: every test leg on this head is pending (build and Documentation green on the previous head). On e536eeb the CPU, CUDA and AMDGPU legs passed, the oneAPI leg failed on the 80 test_dense assertions this round fixes, and test-metal was cancelled at the 6-hour limit as on every earlier head; on 5e689a6 the CPU leg passed and the GPU legs were superseded by this push. Since the core is back to e536eeb, the CUDA/AMDGPU outcome should repeat; the oneAPI leg on this head is what confirms the fix (the implementer's run of test_dense, test_numeric_cholesky_c, test_api on headroom passed: 44,231 / 0 / 4 broken). The owner should wait for the oneAPI leg before merging.

Blocking findings: none.

Nits (non-blocking, inline): (1) ext/SparseDirectSolverOneAPIExt.jl:234: the "oneMKL follows potf2" claim is not distinguished by the tests, which all set A[j, j] = -1 before the factorization; one case where the pivot only turns non-positive during elimination would pin it (explicit :vendor path only). (2) TASKS.md:3384: say explicitly that test-metal has not completed on any head, name the commit of each headroom run, and consider a timeout-minutes on the Metal job.