Skip to content

Preserve StaticArray container types in parameter and integral caches - #617

Draft
ChrisRackauckas-Claude wants to merge 7 commits into
SciML:masterfrom
ChrisRackauckas-Claude:agent/597-parameter-caches
Draft

ChrisRackauckas-Claude wants to merge 7 commits into
SciML:masterfrom
ChrisRackauckas-Claude:agent/597-parameter-caches

Conversation

@ChrisRackauckas-Claude

@ChrisRackauckas-Claude ChrisRackauckas-Claude commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

Please ignore until reviewed by @ChrisRackauckas.

Derived p and I caches were always materialized as Vector via UnitRange broadcast / typeof(x)[], so static-array inputs could never keep static caches. Constructors now map into the input container family (SVector/similar_type for static t, Vector otherwise), empty caches use length-0 static containers when appropriate, and invert_integral materializes static I (or copies an existing cache) before pushfirst! while reusing a freshly generated dynamic Vector.

Multi-field ParameterCaches use separate type parameters per coefficient field so Unitful / mixed-precision inputs that master accepted still construct. Cached plain-Vector cumulative integrals use one-buffer generator cumsum; static knots keep the SVector segment+cumsum path.

Fail-before / pass-after

StaticArray caches (#597)

Command (master sources, new assertions):

julia +1.12 --project=. -e 'using Test, DataInterpolations, StaticArrays;
u=SA[1.0,2.0,4.0]; t=SA[0.0,1.0,2.0];
A=LinearInterpolation(u,t; cache_parameters=true);
@test A.p.slope isa SVector{2,Float64}; @test A.I isa SVector{2,Float64}'

Fail-before (origin/master sources): Vector caches.
Pass-after: LinearParameterCache{SVector{2, Float64}} / SVector{2, Float64}.

Round-2: Unitful / mixed-precision / Vector I construct alloc

On 87f2e2e: Unitful default constructors and mixed-precision Hermite raised MethodError; cached Vector construct alloc was +50%/+100% vs master. Fixed in cfa4225.

Round-3: invert_integral Vector copy

On cfa4225, get_I unconditionally collect(I). Default-cache Vector invert_integral at 10k knots allocated ~349,912 B vs master 269,872 (+30%). After b740085: ~269,840 B (<= 270_000); static inverse and cached-A.I preservation checks pass.

Verification

export TMPDIR=.../jobs/597-cursor/tmp
# Runic + typos on touched files: both exit 0
julia +1.12 --project=@runic -e 'using Runic; exit(Runic.main(ARGS))' -- --check \
  src/integral_inverses.jl test/qa/alloc_tests.jl test/Methods/integral_inverse_tests.jl
typos <same files>

GROUP=Core    julia +1.12 --project=. -e 'using Pkg; Pkg.test()'  # exit 0
GROUP=QA      julia +1.12 --project=. -e 'using Pkg; Pkg.test()'  # exit 0 (alloc_tests 20)
GROUP=Methods julia +1.12 --project=. -e 'using Pkg; Pkg.test()'  # exit 0 (integral_inverse 320)

Not verified

  • Full interpolant isbits (still blocked by FindFirstFunctions.Guesser.idx_prev::RefValue{Int})
  • CubicSpline h/z remain Vector for SVector inputs (only p/I addressed)
  • GPU / DiffEqGPU EnsembleGPUKernel end-to-end
  • Online push!/append! with static caches (still Vector-oriented)
  • Pre-existing QuadraticSpline integral discrepancy (independent review; not introduced here)

Reviewer push-back

  • Whether empty caches should be nothing instead of length-0 SVector
  • Whether CubicSpline should also keep static h/z
  • Whether cumsum on SVector segment integrals is the preferred API vs building an SVector of prefix sums directly
  • Perf of ntuple/map vs prior broadcast for large dynamic t

Part of #597

Please ignore this draft until reviewed by @ChrisRackauckas.

Risk assessment

Risk: medium
Blast radius: all interpolants that build p / I caches (Linear, Quadratic, QuadraticSpline, CubicSpline, SmoothedConstant, Hermite variants); invert_integral via get_I; constructor type inference for empty vs filled I; multi-field coefficient type parameters.
Evidence: fail-before/pass-after on SVector caches; Unitful + mixed-precision + Vector construct-alloc + invert_integral-alloc regressions green; GROUP=Core/QA/Methods exit 0 on b740085.
Independent review: Codex (gpt-6-astra) rated it medium, high confidence: round 4 MERGE, no findings — Core 18,028 pass, QA 20+20, Methods 94,254 (320 integral-inverse); invert_integral allocation identical to master in all 8 cases; new allocation test fails on the previous head and passes; 240/240 ownership probes. Medium because cache representations change across multiple interpolants (no documented type-parameter contract broken).
Merge: needs human review (reviewer rated medium; Part of #597).

Harness: Cursor Agent CLI 2026.10.01-14929f9; model: auto; transcript: /log.txt

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

Risk assessment (independent review, Codex gpt-6.1-sol, head b740085)

  • Verdict: CHANGES
  • Risk: medium-high (MH1: changes types and inference through the exported LinearInterpolation constructor and its default evaluation path, a widely used entry point in a core package; the confirmed regression affects static inputs)
  • Blast radius: Parameter-cache construction across linear, quadratic, smoothed-constant, spline and Hermite interpolants; cumulative integration and inverse integration. Ordinary dynamic-vector tests passed, but constructing and evaluating a static-input interpolant inside another function now loses its concrete inferred result type.
  • Evidence: Read pr.txt first, all of risk.md, and the full git diff origin/HEAD...HEAD; compared head b740085 with base b98db80. All Julia commands used Julia 1.13.2, JULIA_NUM_THREADS=4, the requested JULIA_DEPOT_PATH, and timeout 3600; sources and temporary environments were confined to scratch. Ran julia --project=. targeted.jl (test/parameter_tests.jl and test/Methods/integral_inverse_tests.jl): 416 assertions passed, including 63 static-cache assertions and source-cache preservation. Ran julia --project=base regression.jl: both new SVector cache assertions failed; the corresponding assertions passed on head in targeted.jl. Ran julia --project=base probe.jl and julia --project=. probe.jl: base constructor inference passed and composed evaluation inferred Float64; head constructor inference failed and composed evaluation inferred Any. Ran analytic.jl: 16 analytic/ForwardDiff assertions passed. Ran alloc-new.jl: all four new allocation assertions passed. Ran promoted-inference.jl: both strengthened matrix inference assertions passed. git diff --check passed and the original checkout remained clean. Logs and reproductions are in scratch. G1 PASS: independent analytic linear values/integrals and Unitful analytic tests passed. G2 NOT APPLICABLE to specialized GPU/AD-rule/Reactant/MPI edits (none changed); ForwardDiff through evaluation and constructor parameters was additionally exercised. G3 PASS: cache container/content tests and inverse-integral cache preservation test passed.
  • Merge: needs changes (restore static-input constructor inference and add an inference regression test; supplied metadata also records an unresolved CHANGES review on this head)

🤖 Posted by an AI agent — harness: Codex CLI 0.162.1 · model: gpt-6.1-sol
Conversation: local headless review run (no public session URL); transcript kept by the fleet master.

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

Risk assessment (independent review, Claude Opus 5.5 (claude-opus-5-5), head b740085)

  • Verdict: CHANGES
  • Risk: medium (M1: existing calls now return different types. For static inputs, the p/I fields of LinearInterpolation(SVector, SVector) change from Vector to SVector. The 4th type parameter of the exported QuadraticInterpolation, QuadraticSpline, CubicSpline, CubicHermiteSpline and QuinticHermiteSpline changes from the coefficient vector type to the cache struct type. The *ParameterCache structs go from 1 type parameter to 2 or 3. Changing what an existing call returns is a design call for Chris. It is also close to H1 if anyone downstream names these type parameters; I could not check that without GitHub search.)
  • Blast radius: Every cached constructor in src/parameter_caches.jl and _cumulative_integral (Linear, SmoothedConstant, Quadratic, QuadraticSpline, CubicSpline, CubicHermite, QuinticHermite, plus I caches for all methods), and get_I/invert_integral. With Vector inputs the values are unchanged (checked below). The only visible change there is the struct type parameters, e.g. QuadraticParameterCache{Vector{Float64}} becomes QuadraticParameterCache{Vector{Float64}, Vector{Float64}}. With static-array inputs (SVector/MVector), cache types change and constructor inference regresses. DataInterpolations is a core package, but static-array inputs are a peripheral lane. The main MTK/OrdinaryDiffEq lane (Vector inputs) keeps its values and inference.
  • Evidence: All runs on Julia 1.12, from git archive copies of master b98db80 and head b740085 in scratch.
    • Fail-before / pass-after: the new test/parameter_tests.jl against master gives params | 55 Pass 42 Fail (the StaticArray testset fails at parameter_tests.jl:86/87/113). On head, Core/parameter_tests.jl passes 97/97.
    • Head groups: GROUP=Core passes (interpolation_tests 17573 pass / 3 broken, the 3 broken are pre-existing). GROUP=Methods passes (derivative 75385, integral 18498, integral_inverse 320, online 30, thread_safety 28). GROUP=Extensions passes (Mooncake 769, Zygote 29470, SparseConnectivityTracer 660). GROUP=QA passes (alloc 20, qa 20).
    • Vector-input values: I printed all p fields and I for Quadratic, CubicSpline, QuadraticSpline, SmoothedConstant, CubicHermite and QuinticHermite with cache_parameters=true. The output on master and on head is digit-for-digit identical.
    • The two flipped @test_broken→@test @inferred (Vector{Matrix} Linear and Quadratic) are false on master and true on head, so that is a real improvement.
    • Inference regression (scratch/claude/probe.jl), @inferred M(SA[1.0,5,3,4], SA[1.0,2,3,4]) with default kwargs: true on master and false on head for Linear, Quadratic, CubicSpline, QuadraticSpline, SmoothedConstant and Constant. Base.return_types(g, (SVector{4,Float64}, SVector{4,Float64}, Float64)) with g(u,t,x) = LinearInterpolation(u,t)(x) gives Any[Float64] on master and Any[Any] on head. Vector inputs stay inferable.
    • G1 (numerics): not applicable, because Vector results are bit-identical. The static-path tests compare against the Vector implementation, a different code path, and the Unitful tests check analytic values (1 + t).
    • G2 (AD paths): pass. The Mooncake and Zygote extension tests ran locally on head and passed (counts above).
    • G3 (shared state/caches): pass. test/Methods/integral_inverse_tests.jl checks that invert_integral does not mutate an existing A.I cache, which is the stale-cache risk get_I introduces.
  • Merge: needs changes (finding 1 blocks. The type-parameter change in finding 2 is Chris's design call at medium regardless).

🤖 Posted by an AI agent — harness: Claude Code · model: claude-opus-5-5
Conversation: local headless review run (no public session URL); transcript kept by the fleet master.

ChrisRackauckas and others added 7 commits October 2, 2026 02:17
Static-array interpolants were forced through UnitRange broadcast and
empty Vector constructors, so p and I could never be isbits even when
u and t were. invert_integral now copies I into a Vector before
pushfirst! so SVector caches stay usable.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Cursor Agent <noreply@cursor.com>
Agent-Harness: Cursor Agent CLI 2026.10.01-14929f9
Agent-Model: auto
Agent-Session: local session, transcript at /home/crackauc/sandbox/goals/issue-backlog/ibd/DataInterpolations.jl/jobs/597-cursor/log.txt on amdci2.julia.csail.mit.edu
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Cursor Agent <noreply@cursor.com>
Agent-Harness: Cursor Agent CLI 2026.10.01-14929f9
Agent-Model: auto
Agent-Session: local session, transcript at /home/crackauc/sandbox/goals/issue-backlog/ibd/DataInterpolations.jl/jobs/597-cursor/log.txt on amdci2.julia.csail.mit.edu
Compute one integral sample before branching so empty and filled
cumulative caches share typeof(sample), restoring concrete inference
for matrix / vector-of-vectors LinearInterpolation.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Cursor Agent <noreply@cursor.com>
Agent-Harness: Cursor Agent CLI 2026.10.01-14929f9
Agent-Model: auto
Agent-Session: local session, transcript at /home/crackauc/sandbox/goals/issue-backlog/ibd/DataInterpolations.jl/jobs/597-cursor/log.txt on amdci2.julia.csail.mit.edu
Prototype empty I without indexing t[2] when length(t)<2 (ConstantInterpolation
single-point case), and promote Vector{Matrix} @inferred checks from
@test_broken now that constructor inference is stable.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Cursor Agent <noreply@cursor.com>
Agent-Harness: Cursor Agent CLI 2026.10.01-14929f9
Agent-Model: auto
Agent-Session: local session, transcript at /home/crackauc/sandbox/goals/issue-backlog/ibd/DataInterpolations.jl/jobs/597-cursor/log.txt on amdci2.julia.csail.mit.edu
The length(t)<2 empty-cache prototype used u*zero(t), which kept Int for
integer knots while real integrals promote to Float64 and broke @inferred
on UnitRange. Match master's empty-generator element type instead.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Cursor Agent <noreply@cursor.com>
Agent-Harness: Cursor Agent CLI 2026.10.01-14929f9
Agent-Model: auto
Agent-Session: local session, transcript at /home/crackauc/sandbox/goals/issue-backlog/ibd/DataInterpolations.jl/jobs/597-cursor/log.txt on amdci2.julia.csail.mit.edu
Multi-field ParameterCaches no longer force a shared pType, restoring
default Unitful and mixed-precision Hermite construction. Cached Vector
cumulative integrals accumulate with a single generator again while
SVector knots keep the static segment+cumsum path.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Cursor Agent <noreply@cursor.com>
Agent-Harness: Cursor Agent CLI 2026.10.01-14929f9
Agent-Model: auto
Agent-Session: local session, transcript at /home/crackauc/sandbox/goals/issue-backlog/ibd/DataInterpolations.jl/jobs/597-cursor/log.txt on amdci2.julia.csail.mit.edu
Unconditional collect doubled the integral buffer for default-cache
Vector knots. Reuse the owned Vector from cumulative_integral, collect
only static caches, and still copy an existing A.I before pushfirst!.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Cursor Agent <noreply@cursor.com>
Agent-Harness: Cursor Agent CLI 2026.10.01-14929f9
Agent-Model: auto
Agent-Session: local session, transcript at /home/crackauc/sandbox/goals/issue-backlog/ibd/DataInterpolations.jl/jobs/597-cursor/log.txt on amdci2.julia.csail.mit.edu
@ChrisRackauckas

Copy link
Copy Markdown
Member

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

Independent review (Devin CLI 3000.11.3, model fusion-claude-opus-5-5-high-sidekick-swe-2-medium): CHANGES, risk medium.

Full review

VERDICT: CHANGES
RISK: medium

Reviewed head b740085 against origin/master b98db80. The PR does what #597 asks for: p and I caches now stay in the static container family, and with Vector inputs every cached number comes out identical to master. Two regressions block it. Both were confirmed by running, and neither is covered by the PR's tests or verification.

Blocking findings

  1. [confirmed by running] Static-array constructors are no longer type-inferable, so code that builds and uses an interpolant in one function now infers Any.

    • Where: src/parameter_caches.jl:15-21 (_empty_cache), every *ParameterCache(..., cache_parameters) constructor, and src/interpolation_utils.jl:428-431.

    • Cause: the runtime cache_parameters::Bool now picks between SVector{0,T} and SVector{N-1,T}. Those are different types, so the struct's IType/pType parameters can't be inferred. On master both branches gave Vector{T}, so inference succeeded.

    • What I ran: scratch/probe_inf.jl, which calls @inferred on the constructors and runs Base.return_types on g(u,t,x) = LinearInterpolation(u,t)(x) with u4 = SA[1.0,5,3,4], t4 = SA[1.0,2,3,4].

      Env @inferred constructors (SVector input, default or cache_parameters=true) return_types(g)
      master, Julia 1.13 ok for Linear, Quadratic, CubicSpline, Constant, CubicHermite [Float64]
      master, Julia 1.10 ok for all five [Float64]
      PR, Julia 1.13 FAIL for all five [Any]
      PR, Julia 1.10 FAIL for all five [Any]

      Vector inputs still infer on the PR. Writing cache_parameters=true as a literal does not help, because the constant isn't propagated that far.

    • Why it matters: Derived caches (p, I) are hard-coded Vector, so static-array-backed interpolants are never isbits #597 wants static interpolants for GPU and kernel use, where inference matters most. Today every SVector user who constructs inside a function gets dynamic dispatch on every call after construction. Calling an interpolant that has already been built is still inferred and allocation-free (scratch/probe_eval.jl: 0 bytes and inferred on both master and PR).

    • Required change: make the cache type depend only on the input types. Two options:

      • For StaticArray knots, always build the filled SVector{N-1} cache whatever cache_parameters says. It costs O(N) once at construction, and cache_parameters would then only control whether evaluation reads the cache.
      • Make the empty-vs-filled choice at the type level instead.

      Either way, add a regression test that calls @inferred on static-input constructors with both cache_parameters values, plus a construct-then-evaluate inference check like g above. The PR's new tests only check isa SVector on values that are already built, so they cannot catch this.

  2. [confirmed by running] The new invert_integral allocation test fails on Julia 1.13, which is CI's "1" QA lane. The PR's own allocations match master; the threshold is the problem.

    • Where: test/qa/alloc_tests.jl:161, @test min_inverse_bytes(A) <= 270_000.
    • The threshold was fitted to one observation: the PR body reports ~269,840 B on 1.12. That leaves under 0.1% headroom, and the comment "Master budgets ~270kB" points to the same origin. QA runs on ["lts", "1"] (test/test_groups.toml), and "1" is 1.13 here.
    • What I ran:
      • julia +1.13 --project=envs/pr13 runfiles.jl pr qa/alloc_tests.jl gives Evaluated: 280064 <= 270000, failing for both Linear and Constant.
      • scratch/probe_alloc.jl measures master and PR at 280,256 B (1.13) and 227,064 B (1.10).
    • Conclusion: the test fails on master code as well. It measures how the Julia version allocates, not the PR.
    • Required change: derive the bound from the buffers the code is supposed to allocate. A Linear/Constant inverse at n = 10 000 should allocate a small fixed number of n-length Float64 buffers. Express the bound as k * 8n plus a stated slack. The test must stay able to catch the extra full-size copy it guards against (+80 kB), which is ≈29% at 1.13's ~280 kB and ≈35% at 1.10's ~227 kB. A ratio between two measurements made in the same process would also work, for example compared against @allocated cumulative_integral(A, true). Then run QA on both lts and 1.13.

    The construction allocation test next to it (< 200_000 / < 120_000) has more headroom: 164,112 / 82,128 on 1.13 and 160,240 / 80,192 on 1.10. It passes, but it was fitted the same way.

Non-blocking findings

  • [confirmed] Mixed-precision Hermite caches now have an abstract eltype. CubicHermiteSpline(Real[1.0, big"1.0", 0.5], ...; cache_parameters=true) stores c₁::Vector{AbstractFloat}, where master stored Vector{BigFloat}, because _unzip_parameters uses map instead of promoting through collect(tuple). The values are identical (scratch/cmp_m13.txt vs cmp_pr13.txt). This only affects performance on unusual inputs.

  • [confirmed] Two eltype changes for the better with Unitful inputs:

    • With cache_parameters=true, the Unitful caches now have concrete per-field eltypes. Master had abstract Quantity{Float64}.
    • The empty β/c₂/c₃ caches now carry the correct units. Master reused typeof(α) for all of them.

    Apart from these eltype changes, every cached value, evaluation and integral in the master-vs-PR dump agrees exactly. The dump covers 28 Vector, Int, Float32, range, matrix, vector-of-vectors, Unitful and mixed-precision cases.

  • [confirmed] Compile time grows with N. ntuple(f, Val(N-1)) compiles once per N: about 4 s each for LinearInterpolation and CubicSpline at SVector{200}, versus 0.15 s at N = 16. That is acceptable given StaticArrays' own size guidance, but it shows up in the probe.

  • [from reading] Comment hygiene (CLAUDE.md):

    • # Compute parameters once to infer types was deleted in six constructors, yet the code below still does exactly that.
    • The comment explaining why _cumulative_integral computes sample was deleted, yet sample is still computed. It is now also computed on the cache_parameters=true path, where it is unused.
    • The new test comments ("Master budgets ~160kB…", "~270kB") describe history and measurements instead of what the code is.
  • [from reading] Inconsistent type parameters. The 4th parameter of QuadraticInterpolation, QuadraticSpline, CubicSpline and the Hermite structs changes from the coefficient vector type to the whole cache type (pCacheType). LinearInterpolation keeps LinearParameterCache{pType}. These parameters aren't documented API and nothing in ext/ depends on them (I grepped), but the change is inconsistent and could surprise downstream code that dispatches on them.

  • [confirmed] MVector and SizedVector inputs produce MVector / SizedVector caches. Construction, evaluation, integration and invert_integral all work. Only SVector inputs are isbits.

  • [confirmed] The @test_broken → @test flips in test/interpolation_tests.jl (Vector{Matrix} Linear/Quadratic @inferred) hold. interpolation_tests.jl passes on 1.10 (17,568 pass, 8 broken) and on 1.13. These are genuine fixes, not hidden ones.

What I ran

All runs used the PR tree at scratch/pr and the master tree at scratch/master, both from git archive, in dev'd envs under scratch/envs.

  • Targeted tests, PR, Julia 1.13: parameter_tests.jl (97/97), Methods/integral_inverse_tests.jl (320/320), Methods/online_tests.jl (30/30), qa/alloc_tests.jl (18 pass, 2 fail; finding 2).
  • PR, Julia 1.13: interpolation_tests.jl + Methods/integral_tests.jl + Methods/derivative_tests.jl: 111,456 pass, 3 broken.
  • PR, Julia 1.10: interpolation_tests.jl: 17,568 pass, 8 broken.
  • Extensions group, PR, Julia 1.13 (not run by the author): zygote_tests.jl + mooncake_tests.jl + sparseconnectivitytracer_tests.jl, 30,899/30,899 pass. This matters because Zygote and Mooncake differentiate through the rewritten cache constructors.
  • Fail-before: the PR's parameter_tests.jl against master code fails 42 assertions in "StaticArray parameter and integral caches". The Unitful and mixed-precision testsets also pass on master; they guard against the author's own intermediate commits.
  • Probes:
    • probe1.jl: issue reproducer and isbits checks on p/I, plus MVector/SizedVector, large N, SVector{SVector} u, ForwardDiff through static constructors (gradients match the Vector path), range t, 2-knot input.
    • probe_cmp.jl: master vs PR value dump.
    • probe_inf.jl: constructor inference.
    • probe_eval.jl: evaluation and integral inference and allocations.
    • probe_alloc.jl: allocations on 1.10 and 1.13, master and PR.
  • Formatting and spelling: Runic --check on all nine touched files exits 0; typos src test exits 0. This reproduces the PR's claim.
  • Allocation claim: reproduced in substance. The PR's invert_integral and construction allocations equal master on both 1.10 and 1.13. The absolute threshold in the PR does not hold on 1.13.

What I did not verify

  • Full GROUP=Core/QA/Methods runs through Pkg.test (host limits). I ran the touched and affected files directly. I did not run Aqua, JET or ExplicitImports, and Julia 1.12 was not used.
  • The 32-bit Core lane, the pre channel, and the docs build (no docstring changes).
  • GPU / DiffEqGPU end-to-end use. Full interpolant isbits is still blocked by FindFirstFunctions.Guesser, as the PR says.
  • Online push!/append! with static caches; they are not meant to work for static inputs.
  • No sibling PRs were linked from Derived caches (p, I) are hard-coded Vector, so static-array-backed interpolants are never isbits #597, so I did no interface-fit comparison.

🤖 Posted by an AI agent — harness: Devin CLI 3000.11.3 (review), Claude Code 2.1.285 (posting) · model: fusion-claude-opus-5-5-high-sidekick-swe-2-medium
Conversation: local Claude session 3c311569-dda6-4687-bc9f-65febb212c33 (fleet master)

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member Author

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

Independent review (Codex CLI 0.166.0 / gpt-6.1-sol), session 01a19810-b83a-71c2-a261-3f6ff538aa08, reviewed head b740085

  • Verdict: CHANGES
  • Risk: medium (M3: static cache construction changes hot-path inference; dynamic-vector benchmarks do not cover this path).
  • Round / slot: 1 / 1
  • Blocking priorities: P2

Review written by the reviewer named above in a session other than the PR author's, and copied here by the posting agent without change to its findings. The posting agent did not re-run the reviewer's commands.

Summary: The PR preserves static parameter and integral caches and separates coefficient field types across interpolation constructors. The added tests pass, but static construction followed by evaluation loses concrete inference and needs repair.

Blast radius: Parameter caches for Linear, SmoothedConstant, Quadratic, QuadraticSpline, CubicSpline, CubicHermite and QuinticHermite interpolation; cumulative integrals and inverse-integral cache ownership. The identified defect affects static-input construction and subsequent composed evaluation.

Evidence: Head focused tests pass 421/421 on Julia 1.13.2; static composed evaluation infers Any. Base gives 379 passes and 42 expected static-cache failures; head passes all 421. The identical base inference probe returns a concrete constructor and Float64 evaluation; the independent analytic/ForwardDiff probe passes 382/382. G1 has independent affine and polynomial references, G2 has CI AD/tracing execution plus local static ForwardDiff, and G3 has inverse-cache preservation coverage. All live CI checks pass, including exercised Mooncake, Zygote and sparse tracing tests on this exact head.

Blocking findings

  1. [P2] Restore concrete inference when static caches have different lengths — src/parameter_caches.jl:31 and src/interpolation_utils.jl:431 (confirmed by running). With u = SA[1.0, 2.0, 4.0], t = SA[0.0, 1.0, 2.0], and f(u,t,x) = LinearInterpolation(u,t)(x), Base.return_types(f, Tuple{typeof(u),typeof(t),Float64}) gives Any[Any] at head versus Any[Float64] on base with the same dependency versions. The constructor itself infers a partially specified LinearInterpolation with unknown integral and parameter-cache types, despite default cache_parameters=false. Differently sized static caches make the Boolean branches produce different concrete types; preserve constant propagation or specialize cache builders so a fixed option yields concrete constructor and evaluation types. Add @inferred regression coverage for construction and composed evaluation, including both cache settings.

Non-blocking findings

None.

What I ran

  • Read the full diff and surrounding construction, integration, evaluation, online mutation and extension callers before recording medium (M3) in INITIAL.md; ran sha256sum INITIAL.md > INITIAL.sha256 before fetching author context.
  • Git: git fetch origin '+refs/pull/617/head:refs/remotes/pr/617' master; git checkout --detach pr/617; git rev-parse HEAD; git diff origin/master...HEAD; git diff --check; git merge-base origin/master HEAD. Commands exited 0; merge-base equals the Base above.
  • Julia run 1: timeout 3600 julia +1.13.2 --startup-file=no targeted.jl src env-head (exit 0). The scratch runner activates an isolated environment, develops the selected checkout, and loads Test, StaticArrays, Unitful and FiniteDifferences. It includes the head versions of test/parameter_tests.jl, test/Methods/integral_inverse_tests.jl, and only the new allocation testsets at test/qa/alloc_tests.jl:124. Tail: Changed regression files | 421 421. Inference output: constructor has free IType,pType; composed evaluation Any[Any]; computed value 1.5.
  • Julia run 2: timeout 3600 julia +1.13.2 --startup-file=no targeted.jl base env-base (exit 1, expected regression-test failures). Ran the identical head test files against the archived merge-base, with the same dependency versions. Tail: Changed regression files | 379 42 421; parameters 55 pass/42 fail, inverse integrals 320 pass, new allocation assertions 4 pass. All 42 failures are new static-container assertions, establishing failing-before/passing-after for the cache-container fix. Unitful, mixed-precision, inverse-preservation and allocation additions already pass on base; they guard against regressions introduced during the PR rather than reproduce base defects. Base constructor is concrete and composed evaluation infers Float64, confirming finding 1 is introduced by head.
  • Julia run 3: timeout 3600 julia +1.13.2 --startup-file=no --project=env-head probe.jl (exit 0). Independent references were the affine function 3+2t and its integral 3t+t^2, plus quadratic, cubic and quintic monomials and their analytic derivatives/integrals. Output tails: affine values/integrals/AD 294/294; mixed static/dynamic and array-valued inputs 16/16; ForwardDiff through static construction 4/4; analytic static inverse/cache isolation 8/8; polynomial accuracy 54/54; quadratic-spline affine values 6/6. Tested SVector, MVector and dynamic-vector affine inputs with both cache settings. Total 382/382; no further Julia runs were made.
  • All Julia runs use JULIA_NUM_THREADS=4, OPENBLAS_NUM_THREADS=1, an isolated temporary directory, and the assigned shared depot followed by default depots. No package-wide suite or Pkg.test() was run.
  • GitHub: gh pr checks 617 --repo SciML/DataInterpolations.jl (exit 0, all checks pass); inspected the exact-head CI run and Extensions job log. Output: Mooncake 769/769, sparse tracing 660/660, Zygote 29470/29470. The first log download failed because the default CLI cache was read-only; using a writable cache succeeded. No failed jobs required a default-branch comparison.
  • Read linked issue Derived caches (p, I) are hard-coded Vector, so static-array-backed interpolants are never isbits #597 and author claims after hashing the blind assessment. Author trailers were extracted from the last non-merge entry of gh api --paginate repos/SciML/DataInterpolations.jl/pulls/617/commits, commit b740085.

What I did not verify

No GPU, MPI, Reactant, other-platform, full downstream or full package suite was run locally; this PR changes no specialized GPU kernel, MPI implementation or AD rule. CI supplies the extension execution evidence. End-to-end GPU suitability remains outside this cache-container change. Static construction timing and complete interpolant isbits status were not measured. I did not rerun the full interpolation test file or directly isolate its two strengthened matrix-inference assertions; the Core CI checks cover that file. Unitful caching with cache_parameters=true was not exercised locally.

Independence limitation: fetching the PR body after the blind assessment also displayed embedded earlier independent-review summaries. Those summaries were seen before this review was frozen; prior/PRIOR.md remained unopened. Findings here require my own execution evidence, but full independence after the initial risk assessment cannot be claimed.

🤖 Posted by an AI agent — harness: Claude Code · model: claude-sonnet-5-5
Conversation: https://claude.ai/code/session_01379heiiB35TtywhR7uSU74

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