Skip to content

Document solving the stiff Brusselator on the GPU with a sparse Jacobian - #908

Open
AJ0070 wants to merge 3 commits into
SciML:masterfrom
AJ0070:gpu-sparse-jacobian-section
Open

AJ0070 wants to merge 3 commits into
SciML:masterfrom
AJ0070:gpu-sparse-jacobian-section

Conversation

@AJ0070

@AJ0070 AJ0070 commented Sep 12, 2026

Copy link
Copy Markdown
Member

Adds a GPU section to the advanced ODE tutorial, after the preconditioner section, since it uses the same Brusselator and the same sparse Jacobian setup.

Two things have to change to run that problem on a device: the right-hand side has to broadcast over an index set that lives on the device, because brusselator_2d_loop uses a scalar for loop, and the jac_prototype has to be a CuSparseMatrixCSR so the Jacobian is built on the device too. Preconditioners work through precs with kp_ilu0 from KrylovPreconditioners.jl.

Checked on a T4 at the tutorial's own (32, 32, 2) state and sparsity prototype: TRBDF2 and FBDF both match the CPU reference, unpreconditioned GMRES to 8e-10 and kp_ilu0 through precs to 6e-8. The blocks are not executed by the docs build, matching the Sundials preconditioner examples on the same page, since CUDA is not a docs dependency.

Requested in SciML/OrdinaryDiffEq.jl#1566, replacing SciML/OrdinaryDiffEq.jl#4453. Needs SciML/OrdinaryDiffEq.jl#4452, which is merged.

AI Disclosure: Used Opus 5

`for` loop, and that cannot run on a device. Writing the same stencil as one broadcast
puts the work on whichever device the index set lives on:

```julia

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This doesn't build / run the example.

@ChrisRackauckas

Copy link
Copy Markdown
Member

🤖 Automated review from an AI agent running as @ChrisRackauckas — not written or reviewed by Chris. It is posted so that you can act on it. Chris or a maintainer may disagree.

Verdict: changes needed before merge (reviewed at c34c882).

Adds executable GPU Brusselator examples using a sparse Jacobian, CUDSS, and ILU-preconditioned GMRES. The execution request is addressed and technical checks passed, but the new MPL-2.0 dependency requires an explicit decision under your copyleft-dependency rule before merging.

Risk assessment

  • Risk: medium
  • Blast radius: Advanced ODE tutorial and documentation environment; no solver implementation or public API changes. KrylovPreconditioners becomes a direct documentation dependency in this MIT-licensed repository.
  • Evidence: All head checks passed: Documentation (30m28s), Runic / Runic Format Check, Spell Check with Typos / Spell Check with Typos, and runic-suggestions / Runic Suggestions. Latest master Documentation and Format Check also passed at ba65857; there are no failing head checks to classify. Read the head documentation log: the V100 runner evaluated all five new GPU blocks, including both solves, without example errors; warnonly does not include :example_block. CI resolved CUDA 6.2.0, CUDSS 0.7.0, and KrylovPreconditioners 0.3.8. Locally, julia --startup-file=no --project=repo check_rhs.jl compared the exact tutorial RHS definitions on the host for initial/random states at four times: Broadcast RHS agrees with tutorial CPU stencil | 8 / 8. Runic 1.11.1 --check --diff gpu_blocks.jl passed (exit 0) on the extracted new example blocks. No local GPU solve or full documentation build: nvidia-smi and /dev/nvidia* are absent. The author's numerical solution-error claims were not independently reproduced. No tests are skipped or weakened; kp_ilu0 is exported and documented in its owning package. No SemVer bump is needed for these tutorial changes.
  • Merge: needs changes (resolve the dependency-policy finding below).

Findings

  1. docs/Project.toml:37 — The new KrylovPreconditioners dependency is MPL-2.0, as verified in the exact v0.3.8 license used by CI; the PR body does not disclose this. Your instructions require new copyleft dependencies to be raised explicitly rather than included silently in routine changes. Document the license and docs-only scope, and obtain Chris's explicit acceptance or choose a permissible alternative before merging. This is a repository-policy gate, not a claim that MPL automatically relicenses DiffEqDocs or that this use is legally incompatible. CUDA.jl and CUDSS.jl themselves declare MIT licenses.

Chris is the only reviewer on this PR; no other maintainer is currently reviewing it. His request to build/run the examples is addressed by the current @example blocks and GPU CI execution. The PR description's statement that the blocks are not executed is stale.

Push a fix and the PR is reviewed again automatically at the new head.


🤖 Posted by an AI agent — harness: Claude Code · model: claude-opus-5-5[1m] (fleet master); review by Codex CLI 0.157.1 / gpt-6-astra
Conversation: local Claude Code session 3cd6500a-1f81-46b5-ac0b-c466e15b6a53 on Chris's Mac (session ID, no URL)

@ChrisRackauckas

Copy link
Copy Markdown
Member

🤖 Automated comment from an AI agent running as @ChrisRackauckas — not written or reviewed by Chris.

PR: #908

Reviewed head: c34c882
Raw SHA-256: 9ed04463b552f2a2af71c2eea4a3bfa0fd0a2eaddaed5222fb1ca7c8ab226c2e
Source SHA-256: 7d8339ab20e3e99b4b67de2102160e62daf9ae6040f559f43196f50c858369ba
Source: standard-review-v1 (harness-log)
Reviewer session: 01a1d150-770e-7d13-92e2-8fc39a6ff587
Slot: 1
Confidence: unknown
Own risk fixed before author rating/discussion: yes

Risk assessment

  • Risk: low (L1)
  • Verdict: MERGE
  • Reviewed head: c34c882

Review report

VERDICT: MERGE
Round: 1
Head: c34c882
Slot: 1
CI: green
Session: 01a1d150-770e-7d13-92e2-8fc39a6ff587

Risk

low (L1: tutorial documentation and docs build dependencies; package runtime APIs are unchanged).

Blocking findings

None confirmed.

  • Verdict: MERGE
  • Round: 1
  • Blocking priorities: none
  • Deferred P2 issues: none

Non-blocking findings

  • docs/src/tutorials/advanced_ode_example.md:405 — Inspection: the RHS comparison only displays a Boolean; neither solve checks its return code. A future mismatch or unsuccessful solve could therefore leave the docs build green. Suggest asserting RHS agreement, including at t = 1.1 when forcing is active, and asserting successful return codes and the final time. The current RHS and preconditioned solve passed the corresponding reviewer probes.

What I ran

  • Reviewer: codex-cli 0.164.0, model gpt-6.1-sol; actual session ID above, no session URL available. Review remained blind to PR text and other reviews. Newest commit has no Agent-Harness/Agent-Model trailers.
  • git diff origin/master...HEAD, git diff --check origin/master...HEAD: no whitespace errors. Final git status --short was empty; head matched the requested SHA.
  • uv run --locked python --version: Python 3.15.0. This Julia repository has no Python project or uv.lock; uv warned that --locked has no effect outside a project. julia --version: 1.13.2; no checked-in Julia Manifest was present.
  • In an archived scratch copy, preserving JULIA_DEPOT_PATH: julia --startup-file=no --project=docs -e 'using Pkg; Pkg.develop(PackageSpec(path=pwd())); Pkg.instantiate(); Pkg.status()' exited 0. Resolved CUDA 6.4.2, CUDSS 0.8.1, KrylovPreconditioners 0.3.8, LinearSolve 5.18.3, OrdinaryDiffEqSDIRK 2.9.8. Inspected dependency licenses: CUDA/CUDSS MIT; KrylovPreconditioners MPL-2.0.
  • CUDA_VISIBLE_DEVICES=1 julia --startup-file=no --project=docs ../gpu_probes.jl: CUDA functional, physical GPU 1, capability 12.0; scalar indexing disabled. Passed 1 GPU smoke assertion, 24 CPU-parity/independent periodic-balance assertions across initial/random/constant states and forcing-switch times, and 2 analytic discrete-Fourier assertions. The documented direct solve then exited 1 with Out of GPU memory; reported effective usage was 100%. The inference service already occupied 96,492 MiB on that card.
  • CUDA_VISIBLE_DEVICES=1 julia --startup-file=no --project=docs ../gpu_ilu_probes.jl: exited 0; repeated those 27 assertions and executed the documented ILU/GMRES solve. Observed ILU_RETCODE=Success t=11.5; asserted success and endpoint.
  • CUDA_VISIBLE_DEVICES=1 julia --startup-file=no --project=docs ../convergence.jl: exited 0; four CPU/GPU comparisons asserted successful return codes. Against the CPU solution at both tolerances 1e-10 with tstops=[1.1], GPU final-state relative differences decreased from 0.0588151 to 0.00175489 when reltol decreased from 1e-3 to 1e-6 (abstol=reltol/1000). CPU differences also decreased, from 0.206866 to 0.00898926. Adding the forcing stop likewise gave decreasing errors. These are diagnostic convergence observations, not an analytic trajectory error bound.
  • julia --startup-file=no --project=. -e 'using Pkg; Pkg.instantiate(); Pkg.test(; julia_args=["--threads=2"])' in the scratch copy: exited 0; ExplicitImports 2/2, Documenter errors 3/3, DiffEqDocs tests passed. Julia jobs used memory scopes, two Julia threads, and four precompile tasks.

What I did not verify

  • Completion of the direct CUDSS solve: local GPU capacity prevented it; this is an environment limitation, not a confirmed PR defect.
  • Full-site Documenter build, V100 hardware, older supported dependency combinations, or an analytic full-trajectory reference. CI green is supplied task metadata, not a CI run I independently inspected.

AUTHOR_GRADE: {"task":"docs","score":4,"difficulty":3,"rounds":1,"author_harness":"unknown","author_model":"unknown","note":"GPU RHS passed parity, periodic balance and analytic Fourier probes; documented ILU solve completed successfully and tighter tolerances reduced error. Package tests passed. Direct CUDSS completion was blocked by local GPU memory; displayed correctness checks could be strengthened into assertions."}

Head link: c34c882


🤖 Posted by an AI agent — harness: codex-cli 0.164.0 · model: gpt-6.1-sol
Conversation: local session 01a1d150-770e-7d13-92e2-8fc39a6ff587 (no session URL)

@ChrisRackauckas

Copy link
Copy Markdown
Member

🤖 Automated comment from an AI agent running as @ChrisRackauckas — not written or reviewed by Chris.

This PR now conflicts with master, which blocks the merge tooling ("base overlap on 'master'"). Could you merge or rebase the current master into the branch and push? The branch allows maintainer edits, so a maintainer can also do it.

The earlier review comment that the example did not build or run was addressed by c34c882, and the latest independent review at that head rated the change low risk with a MERGE verdict. A new push will need a fresh review at the new head before it can merge.


🤖 Posted by an AI agent — harness: Claude Code · model: claude-sonnet-5-5
Conversation: local session 3878b54e-28f2-4a01-a176-a8f5468d1997 (no session URL)

This branch has not been deployed

No deployments
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