Skip to content

Avoid reshape when materialising DualLayout views of matrices - #291

Merged
dlfivefifty merged 1 commit into
masterfrom
dual-matrix-parent
Oct 14, 2026
Merged

dlfivefifty merged 1 commit into
masterfrom
dual-matrix-parent

Conversation

@dlfivefifty

@dlfivefifty dlfivefifty commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

sub_materialize(::DualLayout, V) for a view V = view(A, :, jr) calls _dual_adjoint/_dual_transpose, which do view(parent(V)', jr). This assumes parent(V) is an adjoint/transpose of a vector. But DualLayout is also reported by 1×n matrices that wrap one, e.g. BlockedMatrix(v', ...) or MyMatrix(v'). In that case parent(V)' is an n×1 matrix, so view(·, jr) is linear indexing, and Base reshapes. For infinite arrays this throws:

using BlockArrays, InfiniteArrays, FillArrays
d = BlockedArray(BlockedVector(2.0*(0:∞), (blockedrange(Ones{Int}(∞)),))', (Base.OneTo(1), blockedrange(Ones{Int}(∞))))
d[:, Block.(1:3)] # ArgumentError: Cannot multiply 0 * ℵ₀

Here the reshape's ind2sub calls divrem(i, ℵ₀). This came up indexing a BandedBlockBandedMatrix whose data is such a BlockedMatrix.

The fix adds a _dual_view helper: for a vector it's view(v, jr) as before, and for a matrix it's view(A, jr, only(axes(A,2))). The behaviour for genuine adjoint/transpose vectors doesn't change.

The new tests check that _dual_adjoint/_dual_transpose of a wrapped adjoint/transpose (real and complex) don't produce a ReshapedArray. They also cover the infinite case MyMatrix(InfVec()')[:, 1:3]. Both fail without the fix. The version is bumped to 1.16.1.

🤖 Generated with Claude Code

`_dual_adjoint`/`_dual_transpose` assumed the parent of a `DualLayout` view
is an adjoint/transpose of a vector. When it is a 1×n matrix with
`DualLayout` (e.g. a `BlockedMatrix` wrapping an adjoint vector), the
adjoint is an n×1 matrix and linear indexing into it reshapes, which fails
for infinite arrays. Take a column view instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.81%. Comparing base (4a778aa) to head (1d39d74).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #291      +/-   ##
==========================================
+ Coverage   90.66%   90.81%   +0.15%     
==========================================
  Files          12       12              
  Lines        2068     2102      +34     
==========================================
+ Hits         1875     1909      +34     
  Misses        193      193              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dlfivefifty
dlfivefifty merged commit 05cb55f into master Oct 14, 2026
21 checks passed
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.

1 participant