Repository navigation
T18: FGMRES-IR extension (Krylov.jl), ir_mode = "fgmres" - #83
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| solver = DirectSolver(api_batch_matrix(backend, members, 'L'), sym_structure(T), 'L') | ||
| setparam!(solver, "ir_mode", "fgmres") | ||
| setparam!(solver, "ir_n_steps", 3) | ||
| execute!("analysis", solver, nothing, nothing) |
There was a problem hiding this comment.
nit (non-blocking): the uniform-batch testset runs only (Float64, ComplexF32), while every other batch testset (test/test_ubatch.jl) and the test convention loop over ELTYPES. The per-member value view in _spmv_kernel! is the only batch-specific code and the four-type testset above already covers it with nbatch = 1, so this is fine for now, but looping over ELTYPES here would make the file consistent with the rest of the suite.
| end | ||
|
|
||
| Base.size(op::RefinementOperator) = (op.n * op.ncols, op.n * op.ncols) | ||
| Base.size(op::RefinementOperator, d::Integer) = d <= 2 ? op.n * op.ncols : 1 |
There was a problem hiding this comment.
nit (non-blocking): size(op, d) returns the operator dimension for d <= 0 instead of raising like size(A, 0) does for arrays. Same for FactorPreconditioner below. Krylov.jl never calls it with d < 1, so nothing breaks; d in (1, 2) ? op.n * op.ncols : (d > 2 ? 1 : throw(BoundsError(...))) would match Base semantics.
There was a problem hiding this comment.
VERDICT: APPROVE (head 30231da)
Reviewed T18 (FGMRES-IR extension) against the task text, AGENTS.md and PLAN.md §1.3/§2.5/§2.6.
Scope: only the task's deliverables. PLAN.md untouched, no Manifest.toml, TASKS.md changed only in the T18 section (marker [ ] → [!], Report). ir_mode = "fgmres" and the phase strings are the PLAN names; Krylov is a weak dependency with compat; ext/SparseDirectSolverKrylovExt.jl follows the Metis provider pattern. README and bench/refinement.jl --mode=fgmres are in-scope support.
Tests: the listed criterion test exists (test/test_fgmres.jl): plain IR on the perturbed-pivot KKT system does not gain a digit over 5 steps and stays above 1e-8 after 20, FGMRES-IR reaches ≤ 1e-12 within 20 iterations, for both INTTYPES and all BACKENDS. Element types, multiple right-hand sides, X === B, sub-phase composition, solve_mode 1/2, uniform batch with ubatch_index, interrupt and the missing-provider NotSupportedError are covered. Helpers come from test/utils.jl/test/matrices.jl (ir_solver, ir_solve, SOLVE_SUBPHASES moved there, shared with test_refinement.jl); tol(T), Random.seed!(666). No @test_broken added; the reported count of 148 matches the test bodies.
Conventions / correctness: the new SpMV kernel is a 1-D KA kernel, generic in T/INT, no atomics, no @allowscalar; the preconditioner reuses the permute/sweep/unpermute launches, polls user_host_interrupt, and conjugation for op(A) = conj(M) matches the plain IR path (conj(M)⁻¹ x = conj(M⁻¹ conj(x))). The stacked n·ncols vector layout is consistent between the operator, permute_rhs!/unpermute_solution! (strided-vector path), the Krylov RHS copy (column-major prefix of W.R) and the final X += D. Joint stopping atol = ir_tol · minₖ‖Bₖ‖ bounds every column's relative residual. Per-iteration host synchronization inside the solve and the lazily allocated Krylov basis are inherent to FGMRES and are listed as deviations in the Report.
Report: matches the diff; deviations and test counts are consistent; no follow-up issues needed.
CI: CPU and CUDA jobs are still pending at review time; the Report says CUDA is pending CI, so there is no contradiction. The pipeline handles the outcome.
Blocking findings: none. Two non-blocking nits posted inline (uniform-batch testset covers two element types instead of ELTYPES; size(op, d) for d <= 0).
Closes #18
Task: T18 — FGMRES-IR extension (Krylov.jl)
What was built:
ext/SparseDirectSolverKrylovExt.jl(weak dependency Krylov 0.10) runsKrylov.fgmres!with the factorization as right preconditioner. It plugs in throughFGMRES_PROVIDER, the same pattern as the Metis extension. The core part is insrc/solve/refinement.jl:RefinementOperator(a new KA SpMV kernel over the refinement map),FactorPreconditioner(permute → sweeps → unpermute, with conjugation and interrupt polling) andfgmres_refine!.ir_mode = "fgmres"now drives"solve"/"solve_refinement"; withoutusing Krylovit raisesNotSupportedError.getparam(solver, "ir_n_steps")reports the FGMRES iterations. New tests are intest/test_fgmres.jl.bench/refinement.jlgains--mode=fgmres.Tests:
SDS_TEST_GPU=0 julia --project=. -e 'using Pkg; Pkg.test()': 62532 pass, 1 fail, 1 broken. The failure was a T16 logging test that matched a log message I had changed. I restored the message, and the rerun oftest_refinement,test_aqua,test_fgmresgives 675 pass, 1 broken. Combined: 62533 pass, 0 fail, 1 broken (the existing T16@test_broken). The criterion matrix iskkt_matrix(150, 5, 0)withpivot_epsilon = 1and no pivot pairs. Plain IR stalls on it: 7.8e-5 → 5.9e-5 after 5 steps, 1e-4 after 20. FGMRES-IR reaches ≤ 1e-12 in ≤ 20 iterations. On the MadNLP case1354 K2 dumps, 10 FGMRES iterations give 4e-15–2.6e-12, below the cuDSS matching+IR bar of 4.6e-11, with no matching (table in the Report). CUDA/AMDGPU: pending CI.Deviations from PLAN.md / TASKS.md: FGMRES runs on one stacked vector holding every right-hand side and every active batch member, i.e. the block-diagonal system, with a joint stopping test
‖r‖ ≤ ir_tol·minₖ‖Bₖ‖.ir_n_stepsis the maximum number of iterations, with no restart. FGMRES synchronizes with the host every iteration (Krylov.jl's dot products and norms). An interrupt leaves the unrefined solution inX. The shared test helpersir_solver,ir_solveandSOLVE_SUBPHASESmoved totest/utils.jl.Follow-up issues opened: none
🤖 Generated with Claude Code