Skip to content

Guard β=0 in 0-dim bipermutedimsopadd! and drop empty-array workaround#173

Merged
mtfishman merged 2 commits into
mainfrom
mf/zero-dim-bipermutedimsopadd-beta-false
May 7, 2026
Merged

Guard β=0 in 0-dim bipermutedimsopadd! and drop empty-array workaround#173
mtfishman merged 2 commits into
mainfrom
mf/zero-dim-bipermutedimsopadd-beta-false

Conversation

@mtfishman
Copy link
Copy Markdown
Member

@mtfishman mtfishman commented May 6, 2026

Summary

  • The 0-dim branch of bipermutedimsopadd! read dest[] unconditionally (β * dest[] + α * op(src[])), even when β = 0. By BLAS convention, β = 0 means dest is treated as write-only — its contents need not be defined. With element types whose undef storage is unreadable, the unguarded read crashed: bipermutedimsopadd!(Array{BigFloat, 0}(undef), identity, src, (), (), true, false) threw UndefRefError, since the unassigned slot of a 0-dim array of mutable BigFloat cannot be read. This surfaced via callers that allocate a 0-dim destination via similar and call into bipermutedimsopadd! with β = 0 (e.g. scalar contractions producing a 0-dim result).
  • Adds an iszero(β) guard to the 0-dim branch to skip reading dest in the write-only case. The 0-dim short-circuit itself is kept: it lets downstream array types (e.g. BlockSparseArray{T, 0}) use direct dest[]/src[] accesses instead of having to support getindex on a 0-dim PermutedDimsArray wrapper around their type.
  • The companion isempty(dest) && return dest early return worked around a Strided.jl broadcasting bug fixed by QuantumKitHub/Strided.jl#50, released in Strided 2.3.5. Adds Strided to [deps] (with using Strided: Strided so it isn't reported as stale) and a Strided = "2.3.5" compat floor, then drops the workaround.
  • Adds a regression test covering Array{T, 0} for T ∈ {Float64, BigFloat} with both β = 0 (write-only) and β ≠ 0 (accumulating) cases.

The 0-dim branch read `dest[]` unconditionally (`β * dest[] + α * op(src[])`),
even when `β = 0`. By BLAS convention, `β = 0` means `dest` is treated as
write-only and its contents need not be defined. With element types whose
`undef` storage is unreadable,
`bipermutedimsopadd!(Array{BigFloat, 0}(undef), identity, src, (), (), true, false)`
threw `UndefRefError`, since the unassigned slot of a 0-dim array of mutable
`BigFloat` cannot be read.

The 0-dim case is now handled by the existing multi-dim broadcast path, which
already guards on `iszero(β)`. The previous special case carried a TODO
blocking on `GradedArray` no longer being an alias for `BlockSparseArray`;
that condition is now met (`GradedArrays` ships `AbstractGradedArray <:
AbstractArray` with its own `bipermutedimsopadd!` overload).

The companion `isempty(dest) && return dest` early return worked around a
broadcasting bug in `Strided.jl` fixed by QuantumKitHub/Strided.jl#50,
released in `Strided` 2.3.5. Adds `Strided` to `[deps]` (with
`using Strided: Strided` so it isn't reported as stale) and a
`Strided = "2.3.5"` compat floor, then drops the workaround.

Adds a regression test covering `Array{T, 0}` for `T` in `{Float64, BigFloat}`
with both `β = 0` (write-only) and `β ≠ 0` (accumulating) cases.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented May 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.39%. Comparing base (b4f43b6) to head (6b18f18).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #173      +/-   ##
==========================================
+ Coverage   80.37%   80.39%   +0.02%     
==========================================
  Files          24       24              
  Lines         963      964       +1     
==========================================
+ Hits          774      775       +1     
  Misses        189      189              
Flag Coverage Δ
docs 0.00% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 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.

@mtfishman mtfishman enabled auto-merge (squash) May 6, 2026 22:35
Downstream array types (notably `BlockSparseArray{T, 0}`) rely on the 0-dim
case being handled by direct `dest[]`/`src[]` accesses rather than going
through the permute-broadcast path. Without the short-circuit, the broadcast
path wraps the source in a 0-dim `PermutedDimsArray`, and `getindex` on that
wrapper is undefined for these array types
(`CanonicalIndexError: getindex not defined for PermutedDimsArray{T, 0, (), (), BlockSparseArray{T, 0, ...}}`).

Restoring the special case keeps the BLAS-style `β = 0` guard from the
previous commit so that `Array{BigFloat, 0}(undef)` destinations stay safe.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@mtfishman mtfishman changed the title Drop obsolete 0-dim and empty-array branches in bipermutedimsopadd! Guard β=0 in 0-dim bipermutedimsopadd! and drop empty-array workaround May 7, 2026
@mtfishman mtfishman merged commit 5a0dfab into main May 7, 2026
26 checks passed
@mtfishman mtfishman deleted the mf/zero-dim-bipermutedimsopadd-beta-false branch May 7, 2026 00:38
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