Reject unsorted t in munge_data for matrix/N-d u

[ChrisRackauckas-Claude]

What changed and why

munge_data for vector u checks issorted(t) at construction and raises
a clear ArgumentError for unsorted t. The matrix (AbstractMatrix) and
N-d (AbstractArray{T,N}) overloads in interpolation_utils.jl have no
such check, so an unsorted t is accepted silently and every type that
stores u in matrix/array form returns wrong values without any warning.

This PR adds the same issorted(t; by = ForwardDiff.value) check (and
ArgumentError with the same message used by the vector method) to both
the matrix and N-d munge_data methods.

Failing before / passing after

Before (unmodified master, commit b98db80):

=== candidate 5: matrix u with unsorted t ===
LinearInterpolation([1 2 3 4; 1 2 3 4], [1,3,2,4])(2.5) = [1.75, 1.75]

LinearInterpolation([1 2 3 4; 1 2 3 4], [1, 3, 2, 4])(2.5) silently
returns [1.75, 1.75] on master — t is unsorted and the result is
constructed as if it were sorted, with no error.

New test "munge_data rejects unsorted t for matrix/N-d u" in
test/interpolation_tests.jl constructs the same matrix case plus an
N-d array case and asserts both throw ArgumentError. This test fails
(no error, wrong value returned) against unmodified master and passes
with the fix.

What was not verified

• Did not audit every one of the ~28 munge_data(u, t) call sites across
  interpolation_caches.jl/online.jl individually; they all funnel
  through the two munge_data methods touched here, so the fix applies
  uniformly, but only LinearInterpolation was exercised directly in the
  new test.
• Core/QA groups were run on Julia 1.12 and 1.10 (lts); pre was not
  run.

Anything a reviewer should push back on

The error message text is copy-pasted from the vector method's
non-inversed-argument branch (it does not include the "arguments
inversed" hint, since that heuristic doesn't obviously generalize to
matrix/N-d u). Worth a second look at whether that hint should also
apply here.

Please ignore until reviewed by @ChrisRackauckas.

Risk assessment

• Risk: low
• Blast radius: munge_data(U::AbstractMatrix, t) and
  munge_data(U::AbstractArray{T,N}, t) in interpolation_utils.jl, used
  by every interpolation type's matrix/N-d u constructor. Only rejects
  input that was already silently wrong; no change to accepted (sorted)
  input. No public API signature change.
• Evidence: failing-before/passing-after shown above; new test added.
• Independent review: pending
• Merge: needs human review.

---

🤖 Generated with Claude Code

https://claude.ai/code/session_01LPHREnnonfLg1VcE1EJovv

Breaking change

Breaking: the exported constructors (LinearInterpolation and the others using these overloads) now throw ArgumentError for unsorted time domains that the current release accepts (with wrong results). This is a public behavior change and needs a major bump (Project.toml is still 10.1.3), or Chris's explicit call that it ships as a bug fix. The sorted-domain requirement should also be documented. Flagged by the independent review (codex-cli gpt-6.1-sol).

---

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

🤖 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 fa5bf28)

• Verdict: CHANGES
• Rating: Codex gpt-6.1-sol rated it high (H1) at head fa5bf28
• Risk: high (H1: public constructors reject previously accepted inputs, including a Lagrange interpolation verified against an analytic polynomial; the risk policy explicitly names this PR under H1). L2 and the main-entry-point behavior criterion MH1 also apply, but H1 sets the rating.
• Blast radius: Matrix and N-dimensional constructors using either changed munge_data overload, including LinearInterpolation, LagrangeInterpolation and spline constructors. Sorted ordinary numeric inputs tested here retain their results; callers using unsorted domains now receive an exception, including callers whose full-degree polynomial interpolation was correct.
• Evidence: Read the complete diff using git diff origin/HEAD...HEAD, the PR body/comments, and related issue 427 (the PR has no linked issue). Archived default branch 898d88ec6630d7bcd2886172ac81fa0fcf3e89e7 and head fa5bf2878a6bb818009aa1af1552f860a923ec98 into scratch. Ran python3 scratch/run.py, which sequentially instantiated those copies and ran timeout 3600 julia --project=<scratch/base or scratch/head> <scratch/regression.jl or scratch/probes.jl> using Julia 1.12.4, the requested depot and four threads. The verbatim new testset failed on base: 2 failures, Expected: ArgumentError / No exception thrown; head: 2/2 pass. Analytic linear interpolation and knot-translation ForwardDiff tests: 4/4 pass on each revision. The separate Lagrange probe returned the exact quadratic values on base and ArgumentError on head. Logs and scripts are in scratch/. git diff --check origin/HEAD...HEAD passed. All 21 head check-runs reported success; there was no failing check requiring comparison with the default branch. Read the successful Extensions job log: Mooncake 769/769, Zygote 29470/29470. No dependency, exported-name, skipped-test or tolerance changes occur in the diff; no new dependency-internal access is introduced.
• Evidence: G1 pass: no numerical formula changes; independent analytic linear values and derivatives verify the retained sorted path, and the exact polynomial independently demonstrates the breaking case. G2 pass for affected AD paths: locally exercised ForwardDiff through matrix and 3-D constructor knots; successful CI exercised Zygote/Mooncake tests, including matrix constructors. GPU/Reactant/MPI-specific changes are not present. G3 not applicable: no shared-state, cache, threading or global-default changes.
• Merge: needs changes (resolve H1 release/API compatibility, document the sorted-domain contract, and remove the historical test comment).

---

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

[ChrisRackauckas]

🤖 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 low.

Full review

VERDICT: CHANGES
RISK: low

The code change is correct, and I confirmed the failing-before/passing-after claim by running it. The only blocking item is a three-line test comment that breaks the binding comment rules. Once it is removed, this can be merged.

Blocking findings

1. test/interpolation_tests.jl:2218-2220: the comment narrates history and cites a reference that points nowhere (from reading; nothing to run).

   # The vector `munge_data` method already rejects unsorted `t`; the matrix and
   # N-d methods silently accepted it and returned wrong values
   # (https://github.com/SciML/DataInterpolations.jl, candidate 5).
   

   • "silently accepted it" describes the earlier behaviour. CLAUDE.md says to describe what exists, never what it replaced.
   • "candidate 5" refers to the agent's own private audit list, which CLAUDE.md also forbids ("referencing the current task").
   • The URL is the bare repo root, so it is not a working link to any issue.
   • Required change: delete the comment. The testset name already says what the test checks. If a comment stays, it should state the invariant in the present tense, e.g. "t must be sorted for every u shape". The commit message and PR body are where the history belongs.

Non-blocking findings

• Confirmed by running: the Missing path still skips the check. All three munge_data methods check sortedness only in the branch with no Missing values. If u or t has a Missing eltype, the data is filtered and returned unchecked. On both master and the PR:

  • LinearInterpolation(Um, [1,3,2,4,5,6]) (matrix with a missing) returns [1.75, 3.5] with no error.
  • A Union{Missing,Float64} t with a plain matrix u does the same.
  • The vector method has the same gap (LinearInterpolation(Union{Missing,Float64}[1,2,3,4,5,missing], tu) returns 1.75).

  This gap is older than the PR and the PR matches the vector method, so it does not block. But the PR body says "Only rejects input that was already silently wrong", and it leaves this case unfixed. A follow-up should run the check after filtering, in all three methods.

• Confirmed by running: full-degree matrix LagrangeInterpolation was correct with unsorted t, and now throws. On master, LagrangeInterpolation(U, [1,3,2,4,5,6])(2.5) gave 2.08984375. That equals the exact Lagrange polynomial through those nodes, which I computed independently. With the PR it throws ArgumentError. This matches the vector method, which already rejects unsorted t for Lagrange, so it is consistent. It is still a behaviour change for one accepted input that gave correct results, which the PR's risk section does not mention.

• Confirmed by running: decreasing t with matrix u now throws. LinearInterpolation(U, 6.0:-1.0:1.0) now raises ArgumentError. On master it raised LeftExtrapolationError at evaluation. Issue Interpolation error for monotonically decreasing t #427 shows decreasing t is unsupported anyway, so the clearer error is an improvement.

• From reading: the error construction is copied three times. The same throw(ArgumentError("The second argument (t)…")) block now appears in the vector method (as a format string) and in both array methods. A small helper such as _check_sorted_t(t) would keep the message in one place. This is style only.

• From reading: @test_throws ArgumentError does not check the message. Any ArgumentError would pass, for example the length-mismatch one. In this test the lengths match, so the assertion does fail without the fix (shown below). Checking the message would still make it tighter.

• From reading: textual conflict with sibling PR Reject non-one-based t in munge_data #624. That PR also edits all three munge_data methods (it adds require_one_based_indexing). It also inserts its testset at the same spot, just before @testset "user error". The src hunks are in different places and should merge cleanly, but the test hunks will conflict. Whichever PR lands second needs a rebase. The two interfaces are compatible.

• From reading: no linked issue. The PR refers to "candidate 5" of an unseen audit, so I could not check a parent or tracking issue for other related work.

• Process (attribution rules):

  • The PR body has no 🚧 UNREVIEWED banner as its first line, only "Please ignore until reviewed" partway down.
  • The Agent-Harness: Claude Code commit trailer has no version.
  • Agent-Model: claude-sonnet-5 cannot be checked from here.

What I ran

All runs used Julia 1.11 (julia +1.11). The master copy was made with git archive origin/master into scratch/master. The PR is HEAD fa5bf28. Each had its own dev'd environment.

• New testset before and after the fix (scratch/newtest.jl, the testset body copied word for word):
  • master: 2 failed, both "No exception thrown".
  • PR: 2/2 pass.
  • This confirms the PR's failing-before/passing-after claim.
• Probe script scratch/probe.jl against both master and the PR.
  • Unsorted t with matrix u:
    • On master, these returned wrong values with no error: Linear [1.75, 3.5], Quadratic, Lagrange, Akima, Constant, SmoothedConstant, CubicSpline, CubicHermite, PCHIP, QuinticHermite.
    • On master, QuadraticSpline gave a SingularException.
    • With the PR, every one of these throws ArgumentError with the expected message.
    • BSplineInterpolation and BSplineApprox have no matrix method with these arguments on either branch.
  • Unsorted t with 3-d u (Linear, Quadratic, Constant, CubicSpline, Akima): every one throws ArgumentError with the PR.
  • Sorted inputs still accepted and give the same values as master:
    • t as a plain vector, a range, with repeated knots, as Float32, and as Unitful seconds.
    • 3-d u with a sorted t, and with a Unitful t.
    • ForwardDiff through t (matrix and 3-d both give -7.5) and through u (matrix CubicSpline gives 7.5).
    • A Dual t sorted by value but with decreasing partials is accepted, so by = ForwardDiff.value works.
  • The Missing-path cases and the decreasing-range case are described above.
• GROUP=Core julia +1.11 --project -e 'using Pkg; Pkg.test()' on the PR passed:
  • interpolation_tests: 17573 pass, 5 broken. The diff adds no @test_broken; the 5 were already there.
  • extrapolation_tests: 316 pass.
  • interface: 31 pass.
  • abstract_interface: 11 pass.
  • parameter_tests: 20 pass.
• GROUP=Methods julia +1.11 --project -e 'using Pkg; Pkg.test()' on the PR passed:
  • derivative: 75385 pass.
  • integral: 18498 pass.
  • integral_inverse: 317 pass.
  • online: 30 pass.
  • thread_safety: 21 pass.
• julia +1.12 --project=@runic -m Runic --check --diff on the two changed files reported no changes (exit 0).
• git diff origin/master...HEAD | typos - found nothing (exit 0).
• An independent Lagrange-polynomial calculation for the Lagrange finding gave 2.08984375.
• git status in the checkout is clean afterwards. No Julia process I started is still running.

What I did not verify

• GROUP=QA, Extensions, Misc, Julia lts and pre, and the 32-bit Core job. The PR body says QA passed on 1.12 and 1.10; I did not rerun it.
• Downstream users (ModelingToolkit and others) that might pass matrix u with unsorted or decreasing t, especially full-degree Lagrange. I did not search for them.
• Whether the Symbolics or Reactant paths reach the array munge_data methods with a symbolic t. If they do, issorted could fail to evaluate. The vector method already makes the same call, so I consider this unlikely to be new.

---

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