Bug fixes, broadcasting, matricize, and display improvements#151
Merged
Conversation
scale! now zeros the array when β=0 instead of multiplying (which produced 0*NaN=NaN on uninitialized memory). The iszero(β) guard in permutedimsopadd! for AbelianGradedArray is removed — scale! is always called, correctly handling the β=0 case. Fixes allocating broadcasts like `3 .* a` and `a - a` producing garbage in positions that should be zero. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…atrix broadcasting Generalizes the broadcast style to all AbstractGradedArray types. permutedimsopadd! now dispatches on AbstractGradedArray instead of AbelianGradedArray, giving FusedGradedMatrix scalar multiplication, addition, and fused broadcasting (3 * m, m + n, c .= 3 .* m .+ 2 .* n). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace show(io, label(s)) with show(io, s) in display methods for GradedOneTo, SectorOneTo, and FusedGradedMatrix. This prints U1(0) instead of Irrep[U₁](0). The label() function is an implementation detail that should not appear in user-facing output. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ata) Adds print_array, compact show, and text/plain show for AbstractSectorArray following KroneckerArrays display pattern. Covers both AbelianSectorArray and SectorMatrix. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds FusedGradedMatrix([sector => block, ...]) as a convenience constructor alongside the existing two-vector form. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…Matrix Uses a thin _GradedPrintView that converts GradedOneTo axes to BlockedOneTo for BlockArrays printing infrastructure. Shows dots (⋅) for unstored blocks and block separators (│, ─, ┼) at block boundaries. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…Matrix GradedOneTo now subtypes AbstractBlockedUnitRange (just needs first + blocklasts). Display converts to BlockSparseArray for printing, which provides block separators (│, ─, ┼) and dots (⋅) for unstored blocks. Defines isstored for AbstractGradedMatrix. No scalar getindex added. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- `GradedOneTo` now shows as `gradedrange([s => m, ...])`, with duality on individual sectors rather than a trailing `'` on the whole range — matches a valid `gradedrange` constructor call. - `SectorOneTo` no longer duplicates the trailing `'`; the inner sector already carries the dual marker. - `Base.adjoint(r::SectorOneTo) = dual(r)` for consistency with `GradedOneTo`. - `AbelianGradedArray` summary moves the colon to the first line and prints per-axis "Dim d:" entries in `show(::MIME"text/plain", ...)`. - `AbelianGradedArray` / `FusedGradedMatrix` summaries use `typeof(a)` so ndims aliases (Matrix/Vector) render naturally. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Print duals as `dual(...)` rather than a trailing `'` since `'` is already
Julia's range-adjoint operator (`(1:4)'` gives a 1×N adjoint matrix) and
reusing it for duality is ambiguous. The `dual` wrapper is factored to
the outside for compound types so the printed form round-trips cleanly:
- `dual(gradedrange([s1 => m1, s2 => m2]))`
- `dual(SectorOneTo(s, m))`
- leaves the inner `SectorRange` renderer as `dual(U1(0))`.
- `GradedOneTo` now has a dedicated `MIME"text/plain"` show that injects a
`sectors: [...]` line between the default `AbstractArray` summary and the
block-separated element listing. Dual ranges render as
`sectors: dual.([...])`.
- Generalize `_to_blocksparsearray` (and the `Base.print_array` override)
from `AbstractGradedMatrix` to `AbstractGradedArray{T,N}`. Use
`collect(Array(sector(blk)) ⊗ data(blk))` (via `KroneckerArrays.⊗`),
which supports arbitrary rank instead of the 2D-only `kron`. 3D arrays
now display with the standard `[:, :, k] =` slice-by-slice block layout
inherited from `BlockSparseArray`.
- Implement the minimal `SparseArraysBase` interface on `AbelianBlocks`
(`eachstoredindex`, `storedvalues`, `isstored`) so
`storedlength(blocks(a))` — and by extension `blockstoredlength(a)` —
reflects the dict-of-keys storage. The summary now uses
`blockstoredlength(a)` directly.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Mirror the `AbelianGradedArray` format — print each axis on its own line between the summary and the block body, so the codomain/domain structure (and its duality) is visible without inspecting the sectors list. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #151 +/- ##
==========================================
+ Coverage 86.12% 87.33% +1.20%
==========================================
Files 20 20
Lines 1139 1263 +124
==========================================
+ Hits 981 1103 +122
- Misses 158 160 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…_fused Three suspicious helpers are reduced / removed: - `pad_to_canonical_duals` (`src/fusion.jl`): ~50 lines → ~25 lines. Same algorithm, reorganized: one `Dict` for the canonical-sector position lookup (reused for both codomain and domain remapping), direct `a′[Data(new_i, new_j)] = data(...)` instead of manual `view(...) ∘ copyto!`. - `_pad_fused` (`src/fusion.jl`): removed. The 2-arg axis-aware `AbelianGradedArray(m::FusedGradedMatrix, cod_ax, dom_ax)` constructor now builds the target `AbelianGradedMatrix` directly, so there is no need to first pad `m` with zero-size blocks and then convert. The old 1-arg constructor delegates to the new 2-arg one with `axes(m)...`. - `_mul_mismatched` (`src/fusedgradedmatrix.jl`): removed. `*` is now a three-line wrapper — check-input, allocate, `mul!` — that requires matching sector lists (the standard semantics when the two fused matrices have compatible contracted axes). The check_mul_axes/axes helpers are refactored into the MatrixAlgebraKit-style `check_input(::typeof(op), ...)` so `*` and `mul!` share one source of truth for the A/B axis-compat check. Refresh the `iszero(β) ? … : …` ternary and `one(α)` (instead of the `true` literal) in `permutedimsopadd!`. New tests covering demo-relevant functionality that slipped through the last PR: - 3D `matricize` + `unmatricize` round-trip (the actual path the demo exercises; the existing 2D and 4D tests didn't hit `pad_to_canonical_duals` because their cod/dom axes are already canonical duals). - `FusedGradedMatrix` `Pair` constructor with non-abelian `SU2`. - `FusedGradedMatrix * FusedGradedMatrix` for both abelian (`U1`) and non-abelian (`SU2`) sectors, plus a `DimensionMismatch` test for incompatible sector lists. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mtfishman
changed the title
…emap Rather than unpacking `Int.(Tuple(I))` and indexing back into the precomputed `cod_sects`/`dom_sects` arrays to look up the sector of each stored block, grab a view of the block and pull the sectors off it via `sectoraxes(aI)`. The block already carries its (cod, dom) sector labels, so there's no need to round-trip through integer indices. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
`insert_missing_sectors` is more honest: the function doesn't just "pad to duals"; it inserts multiplicity-0 entries for sectors missing from one axis so that the two axes carry the same (non-dual) sector list. Also add explicit precondition checks — each axis must have sorted, unique sectors (as produced by `sectormergesort`). Repeated sectors would be silently collapsed by the dict-based multiplicity lookup, and unsorted input would be reordered by the internal `sort!`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…er AbstractGradedMatrix Replaces the ad-hoc axis-aware `AbelianGradedArray(m::FusedGradedMatrix, cod_ax, dom_ax)` constructor (which silently dropped m-sectors absent from the target and lived in fusedgradedmatrix.jl) with a named helper in fusion.jl: - `delete_missing_sectors(m::AbstractGradedMatrix, cod_ax, dom_ax)` embeds `m` in target `(cod_ax, dom_ax)` axes, written in terms of block indexing (so it works on any `AbstractGradedMatrix` — tested with both `AbelianGradedArray` and `FusedGradedMatrix`). Preconditions are explicit: - each axis (both `m` and target) must be sorted + unique; - `m`'s codomain/domain must be canonical duals (`cod_m == dual.(dom_m)`); - any sector of `m` absent from either target axis must have multiplicity 0 on at least one side (empty block, no data loss). The target axes themselves are *not* required to be canonical duals — `unmatricize` calls this with the un-padded merged cod/dom, which may have different sector sets. Also: - Add a strict 1-arg `AbelianGradedArray(m::AbstractGradedMatrix)` constructor (keeps `axes(m)` exactly, iterates via `eachblockstoredindex`). - Replace `enumerate` with `pairs` throughout (same package convention as in `insert_missing_sectors`). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… preconditions through `check_input` Matches the pattern already used by `check_input(::typeof(*), A, B)` and `check_input(::typeof(mul!), C, A, B)` on `FusedGradedMatrix`: each sector-axis helper now has a corresponding `check_input(::typeof(fn), args...)` method that holds the precondition checks, keeping the bodies of the helpers themselves focused on the actual work. Forward-declare the two functions (`function f end`) so that `::typeof(f)` resolves on the `check_input` methods preceding the function bodies. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…=0 fix - `AbelianGradedArray(m::AbstractGradedMatrix)`: every allowed block of the target is also in `m`'s stored blocks, so the loop below overwrites every allocation — no `zero!` needed. Add a comment. - `permutedimsopadd!` (graded path): add a comment above the `iszero(β) ? FI.zero!(y) : scale!(y, β)` line explaining why `scale!(y, 0)` isn't equivalent (NaN * 0 == NaN would leak uninitialized garbage from unstored-block slots into allocating broadcasts like `3 * a`). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two `zero!` calls after `similar(...)` were redundant and are removed: - `insert_missing_sectors` (`src/fusion.jl`): every non-empty allowed block of the result corresponds to a stored block of the input (which is a post-`sectormergesort` matrix, so all allowed blocks are stored). Blocks coming purely from padding have multiplicity 0 on one axis — zero-size allocations with no data to clean up. - `Base.permutedims(::AbelianGradedArray, perm)`: the subsequent `permutedims!` → `permutedimsopadd!(β=0)` already zeros the destination internally, so the outer `zero!` was doing the same work twice. Left in place (each annotated with an invariant comment explaining why): - `BlockReshapeFusion` `matricize`/`unmatricize` (zeros allocated dest blocks when input is sparse), - `delete_missing_sectors` (target axes can have sectors absent from m, so some allowed blocks of the result never get written), - Splitting and merging `Base.getindex` on `AbelianGradedArray` (each copy only writes a sub-range of the destination block). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The function is the per-axis inverse of `insert_missing_sectors`: given `m` with canonical-dual-padded axes and target axes that correspond to the un-padded originals, drop the padding entries. The target axes are subsets of `m`'s, per axis. The precondition reduces to three uniform checks applied independently to the codomain axis and the domain axis: 1. Both the input (m) and output (target) axes are sorted and unique. 2. Target sectors are a subset of `m`'s sectors. 3. Sectors in `m` but not in the target have multiplicity 0 (safe to drop, no data loss). Since target sectors are guaranteed to be a subset of m's, every allocated block of the result is overwritten by the loop — no `zero!` needed. Drop the `FI.zero!` and move the invariant comment to the function body next to the `similar` call. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A bundle of fixes, refactors, and display improvements for
AbelianGradedArrayandFusedGradedMatrix.Bug fixes / correctness
β = 0inscale!and drop theiszeroguard inpermutedimsopadd!. Previouslyscale!(y, 0)didn't reliably zeroywhen a block heldNaN/Inf(fromundefmemory), which leaked garbage into unstored-block slots of results from3 * a/a + b. Annotated with a comment at the call site.GradedStyle(renamed fromAbelianGradedStyle) and a genericpermutedimsopadd!path.Features
FusedGradedMatrix([sector => block, ...])Pairconstructor.matricize/unmatricize(viainsert_missing_sectors/delete_missing_sectors).AbelianGradedArray(m::AbstractGradedMatrix)conversion constructor (works for bothAbelianGradedArrayandFusedGradedMatrixinput).Display
SectorRangedisplay:U1(0)instead ofIrrep[U₁](0). For duals, factored wrapperdual(U1(0))(not', which collides with Julia's range adjoint(1:4)').GradedOneToshows asgradedrange([U1(0) => 2, U1(1) => 3])compact; dual form factored todual(gradedrange([...])).MIME\"text/plain\"inserts asectors: [...]/sectors: dual.([...])line between the blocked summary and the element body.SectorOneToshows asSectorOneTo(U1(0), 2); duals factored to the outside asdual(SectorOneTo(U1(0), 2)).AbstractSectorArrayprints as the Kronecker structuresector ⊗ data.AbelianGradedArray/FusedGradedMatrixtext/plain:typeof(a)so theVector/Matrixtype aliases render naturally.Dim d: ...line; block body follows.│ ─ ┼separators and⋅for unstored blocks. Generalized to arbitrary rank viacollect(Array(sector(blk)) ⊗ data(blk))(usingKroneckerArrays.⊗), so 3D arrays display with[:, :, k] = ...slice-by-slice block layout.Internals / cleanup
Several suspicious helpers from an earlier PR were flagged and either simplified or removed:
pad_to_canonical_duals→insert_missing_sectors: renamed to reflect what it actually does (pad each axis with multiplicity-0 entries for sectors missing on that side). Simplified implementation (~50 → ~20 lines) using block views +sectoraxesinstead of integer round-trips. Preconditions (sorted + unique sectors) routed throughcheck_input(::typeof(insert_missing_sectors), ...)._pad_fusedremoved: replaced by a genericdelete_missing_sectors(m::AbstractGradedMatrix, cod_ax, dom_ax)helper insrc/fusion.jl— the inverse ofinsert_missing_sectors. Per-axis set-operation preconditions (sorted+unique, target ⊆ m, m-only sectors are empty blocks) routed throughcheck_input._mul_mismatchedremoved:*onFusedGradedMatrixnow requires matching sector lists (the standard matrix-multiplication semantics). Shared thecheck_mul_axes-equivalent logic between*andmul!viacheck_input(::typeof(*), A, B)/check_input(::typeof(mul!), C, A, B)(MatrixAlgebraKit-style).zero!audit: dropped two redundant calls (inAbelianGradedArray(m)andpermutedims) and annotated the remaining ones with invariant comments explaining when they're needed.GradedOneTo <: AbstractBlockedUnitRange(needsfirst+blocklasts).SparseArraysBaseinterface (eachstoredindex,storedvalues,isstored) onAbelianBlockssoblockstoredlength(a) = storedlength(blocks(a))reflects dict-of-keys storage.nondual(::SectorRange)helper.enumerate→pairsfor consistency.Test plan
Pkg.test(\"GradedArrays\")— all tests pass, including new tests for:FusedGradedMatrixPairconstructor (bothU1andSU2)FusedGradedMatrix * FusedGradedMatrixfor abelian (U1) and non-abelian (SU2) sectors, plus aDimensionMismatchcasematricize+unmatricizeround-trip (exercisesinsert_missing_sectors/delete_missing_sectors+ the new axis-awareAbelianGradedArrayconstructor)GradedOneTo/SectorOneTo/ sector printing (newdual(...)notation)🤖 Generated with Claude Code