Skip to content

T18: FGMRES-IR extension (Krylov.jl), ir_mode = "fgmres" - #83

Merged
github-actions[bot] merged 3 commits into
mainfrom
task/T18-fgmres-ir
Oct 5, 2026
Merged

github-actions[bot] merged 3 commits into
mainfrom
task/T18-fgmres-ir

Conversation

@claude

@claude claude Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Closes #18

Task: T18 — FGMRES-IR extension (Krylov.jl)

What was built: ext/SparseDirectSolverKrylovExt.jl (weak dependency Krylov 0.10) runs Krylov.fgmres! with the factorization as right preconditioner. It plugs in through FGMRES_PROVIDER, the same pattern as the Metis extension. The core part is in src/solve/refinement.jl: RefinementOperator (a new KA SpMV kernel over the refinement map), FactorPreconditioner (permute → sweeps → unpermute, with conjugation and interrupt polling) and fgmres_refine!. ir_mode = "fgmres" now drives "solve"/"solve_refinement"; without using Krylov it raises NotSupportedError. getparam(solver, "ir_n_steps") reports the FGMRES iterations. New tests are in test/test_fgmres.jl. bench/refinement.jl gains --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 of test_refinement,test_aqua,test_fgmres gives 675 pass, 1 broken. Combined: 62533 pass, 0 fail, 1 broken (the existing T16 @test_broken). The criterion matrix is kkt_matrix(150, 5, 0) with pivot_epsilon = 1 and 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_steps is 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 in X. The shared test helpers ir_solver, ir_solve and SOLVE_SUBPHASES moved to test/utils.jl.

Follow-up issues opened: none

🤖 Generated with Claude Code

github-actions Bot and others added 3 commits October 5, 2026 13:59
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>
@claude claude Bot added this to the M5 — Solve extras milestone Oct 5, 2026
@claude claude Bot added the claude:pr PR opened by the Claude implementer; handled by the Claude pipeline label Oct 5, 2026
Comment thread test/test_fgmres.jl
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)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/solve/refinement.jl
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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

@github-actions
github-actions Bot merged commit 3d26819 into main Oct 5, 2026
5 of 7 checks passed
@github-actions
github-actions Bot deleted the task/T18-fgmres-ir branch October 5, 2026 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

claude:pr PR opened by the Claude implementer; handled by the Claude pipeline

Projects

None yet

Development

Successfully merging this pull request may close these issues.

T18 — FGMRES-IR extension (Krylov.jl)

0 participants