Skip to content

Resolve materialize! ambiguity with LazyArrays 2.13 padded ldiv - #91

Merged
ChrisRackauckas merged 6 commits into
SciML:mainfrom
ChrisRackauckas-Claude:devin/padded-ldiv-ambiguity
Oct 4, 2026
Merged

ChrisRackauckas merged 6 commits into
SciML:mainfrom
ChrisRackauckas-Claude:devin/padded-ldiv-ambiguity

Conversation

@ChrisRackauckas-Claude

@ChrisRackauckas-Claude ChrisRackauckas-Claude commented Sep 12, 2026 •

Copy link
Copy Markdown
Member

Ignore until reviewed. This PR was opened by an AI agent and must not be merged until a human has reviewed it.

Summary

LazyArrays 2.13 added materialize!(::MatLdivVec{<:UnitOrUpperTriangularLayout, <:PaddedColumns}) (LazyArrays/src/padded.jl:865), which overlaps this package's MatLdivVec{TriangularLayout{'U',_,AlmostBandedLayout}} methods on the B side. Aqua's ambiguity check on master fails with the pair for both 'U','N' and 'U','U', and a padded right-hand side throws MethodError: ambiguous at runtime.

Add the joint specializations. A padded RHS is zero past its data, so an upper-triangular solve only needs the leading square block of the factor (and the Zeros tail cannot be written into anyway). The prefix is obtained with public ArrayLayouts.colsupport + view, then back-substituted in place using strictly-upper almost-banded entries (banded superdiagonals plus fill rows, never j ≤ k) so storage is O(n) in the prefix length rather than a dense n×n copy. A zero pivot in the stored prefix throws SingularException(k) for non-unit UpperTriangular, matching the dense path; unit-upper solves still ignore the stored diagonal.

LazyArrays.PaddedColumns still appears in the method signatures because that is the layout name that has to match LazyArrays' methods to resolve the ambiguity; it is not public in LazyArrays yet. paddeddata is not used.

In-place ldiv! on Vcat(v, Zeros) uses these methods for all LazyArrays 2.x; out-of-place \ only keeps a padded RHS from LazyArrays 2.13 (older versions copy to a dense Vector first).

Also adds import LazyArrays so the module name is bound (the pre-existing LazyArrays.arguments calls in __arguments were already relying on it).

Test plan

  • QA group reproduces CI failure unfixed: test/qa/qa.jl → 19/20, "Method ambiguity" (2 pairs)
  • With fix: 20/20 pass on Julia 1.13
  • In-place ldiv!(UpperTriangular(A), Vcat(p, Zeros)) matches the dense reference for UpperTriangular and UnitUpperTriangular on LazyArrays 2.0.0 and 2.14.2
  • Out-of-place \ asserted on LazyArrays ≥ 2.13
  • Negative upper bandwidths u=-1 and u=-2, both triangular variants, full-rank fill, vs dense reference
  • Zero pivot in the stored prefix throws SingularException(2) like the dense control
  • qa.jl keeps a qualified-access ignore only for PaddedColumns, with a comment that it is irreducible until LazyArrays declares the layout public

Risk assessment

  • Risk: medium
  • Blast radius: Adds two Base.materialize! methods for upper-triangular AlmostBanded solves with a LazyArrays.PaddedColumns right-hand side. The exported API is unchanged. The signatures must name non-public PaddedColumns to disambiguate; the solve itself uses public colsupport.
  • Evidence: GROUP=Core (93 pass / 2 pre-existing broken) and GROUP=QA (11/11 alloc, 20/20 QA) on Julia 1.13.1 / LazyArrays 2.14.2.
  • Independent review (round 1): Codex gpt-6-astra, CHANGES — non-public access, quadratic densify, inaccurate 2.13-inert claim.
  • Independent review (round 2): Codex gpt-6-astra, CHANGES — negative-bandwidth back-substitution and missing SingularException on zero pivots. This update addresses those.
  • Merge: ignore until reviewed — remaining PaddedColumns qualified access still needs a human.

Agent-Harness: Cursor Agent CLI
Agent-Model: auto
Agent-Session: ~/sandbox/goals/ci-green/work/cig-fabm91-r1/log.txt (local transcript, amdci2)

ChrisRackauckas and others added 6 commits September 12, 2026 13:52
LazyArrays 2.13 added `materialize!(::MatLdivVec{<:UnitOrUpperTriangularLayout,
<:PaddedColumns})`, which overlaps the AlmostBandedLayout upper-triangular
`materialize!` methods on the B side: Aqua's ambiguity check fails with the
pair for both 'U','N' and 'U','U', and the call throws `MethodError:
ambiguous` at runtime.

Add the joint specializations. The padded solve only needs the leading
square block of the triangular factor, since the right-hand side is zero
past its data (and the `Zeros` tail cannot be written into); solve that
block densely.

`import LazyArrays` is also added so the module name is bound — the existing
`LazyArrays.arguments` calls were already resolving through it.

Verified: `test/qa/qa.jl` passes 20/20 on Julia 1.13 (was 19/20 with the two
ambiguities), and `UpperTriangular(A) \ Vcat(p, Zeros)` returns the exact
solution on the padded prefix with a zero tail (previously MethodError).

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Devin <noreply@cognition.ai>
Agent-Harness: Devin CLI 3000.10.21
Agent-Model: SWE-2 Max
Agent-Session: /home/crackauc/.local/share/devin/cli/summaries/history_1130a74e0ebc4347.md
`const` on a local variable is a syntax error; the alias is only used in
the enclosing testset anyway.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Devin <noreply@cognition.ai>
Agent-Harness: Devin CLI 3000.10.21
Agent-Model: SWE-2 Max
Agent-Session: /home/crackauc/.local/share/devin/cli/summaries/history_70c8672a582f4377.md
LazyArrays' `materialize!` for
`MatLdivVec{<:UnitOrUpperTriangularLayout, <:PaddedColumns}` — the padded
right-hand-side path the new methods disambiguate — was added in LazyArrays
2.13, and only then does `Vcat(data, Zeros)` take on a `PaddedColumns` layout.
On the downgrade lane's floor resolution (LazyArrays 2.0.0), `A \ bpad` has no
upstream path and errored with `size(::Nothing, ::Int64)`, turning the
previously-green Downgrade job red. The new `materialize!` methods are
inert on older LazyArrays (the layout never arises), so gate only the test.

Verified locally: the Triangular testset reports 5 pass / 2 broken on
LazyArrays 2.13.0 and 3 pass / 2 broken on LazyArrays 2.0.0 — identical to
the pre-PR state there.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Devin <noreply@cognition.ai>
Agent-Harness: Devin CLI
Agent-Model: SWE-2 Max
Agent-Session: local session, /home/crackauc/.local/share/devin/cli/sessions.db
Drop non-public paddeddata in favor of ArrayLayouts.colsupport, back-substitute
the padded prefix without a dense n×n copy, and document that PaddedColumns
(and in-place ldiv!) already apply on the LazyArrays 2.0 compat floor.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Cursor Agent <noreply@cursor.com>
Agent-Harness: Cursor Agent CLI
Agent-Model: auto
Agent-Session: ~/sandbox/goals/ci-green/work/cig-fabm91-r1/log.txt (local transcript, amdci2)
Restrict back-substitution to strictly-upper columns so u=-1 and u=-2
match the dense triangular solve, and throw SingularException on a zero
prefix pivot instead of returning NaN/Inf.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Cursor Agent <noreply@cursor.com>
Agent-Harness: Cursor Agent CLI
Agent-Model: auto
Agent-Session: ~/sandbox/goals/ci-green/work/cig-fabm91-r1/log.txt (local transcript, amdci2)
On Julia 1.10 the dense UpperTriangular path throws LAPACKException(2)
rather than SingularException(2). Keep the AlmostBanded padded-solve
assertion strict and stop depending on the dense exception type.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
Co-Authored-By: Cursor Agent <noreply@cursor.com>
Agent-Harness: Cursor Agent CLI
Agent-Model: auto
Agent-Session: ~/sandbox/goals/ci-green/work/cig-fabm91-r1/log.txt (local transcript, amdci2)
@ChrisRackauckas
ChrisRackauckas merged commit 06f614a into SciML:main Oct 4, 2026
20 checks passed
ChrisRackauckas referenced this pull request Oct 14, 2026
Agent-Harness: Devin CLI 3000.11.3
Agent-Model: Fusion (Claude Opus 5.5 Medium + SWE-2 Medium)
Agent-Session: local transcript ~/.local/share/devin/cli/transcripts/flash-hour.json

Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
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