Commit dbbf6e2
authored
Add sparse_bidir_logsumexp: simultaneous row- and column-wise log-sum-exp (#89)
* refactor: generalise _scatter_logsumexp to a leading batch dim
Operate on the last axis so values/scatter_index of shape (*batch, nnz)
reduce independently into (*batch, n_groups). The 1-D path is unchanged
(dim=-1 == 0, gather(...,-1,idx) == idx-fancy-index), so sparse_logsumexp's
existing callers and their numerical fixes are untouched; enables a single
batched scatter for the upcoming sparse_bidir_logsumexp.
Suggested-by: tvercaut
* feat: add sparse_bidir_logsumexp (simultaneous row + column LSE)
Computes the column-wise (dim=0) and row-wise (dim=1) log-sum-exp of a 2-D or
batched 3-D sparse tensor in a single traversal, instead of two sparse_logsumexp
passes. Both reductions share one extraction feeding a single batched
_scatter_logsumexp via values.expand(2, nnz) (a view); its backward correctly
sums each nonzero's row+col gradient. Supports COO/CSR/CSC, include_zeros,
keepdim (tuple only), and output_layout in {tuple, padded, nested}; padded is
the native (2, G) buffer.
Suggested-by: tvercaut
* test: cover sparse_bidir_logsumexp
Parametrised over COO/CSR/CSC x value/index dtypes x include_zeros (reusing the
sparse_logsumexp fixtures/helpers): tuple equals the two-call baseline and dense
reference; padded/nested layouts agree and pad to -inf; keepdim shapes and its
rejection for non-tuple layouts; the sparse_logsumexp numerical edge cases (very-negative
values, explicit +inf) exercised on *both* axes; batched shapes; gradient parity
with the two-call baseline; and the error paths.
* bench: add sparse_bidir_logsumexp benchmarks vs two-call baseline
Random-matrix and SuiteSparse (Rothberg/cfd2, Williams/webbase-1M) benchmarks
comparing sparse_bidir_logsumexp against the two-call baseline
(sparse_logsumexp dim=0 + dim=1), mirroring the sparse_logsumexp scripts and
registered in benchmark_suite. Both ops return a single tensor (padded / cat)
so measure_op's backward exercises both reductions. Real >=1M-dim coverage per
the sparse_logsumexp review.
* docs: list sparse_bidir_logsumexp in the README key features
* docs: correct batched equivalence and soften bidir docstring claims
The 'Equivalent to' snippet used dim=0/dim=1, which raises on a batched 3-D
input (batch axis is dim=0); qualify it as 2-D and note the batched dim=1/dim=2
reductions. Also soften the unqualified 'cheaper than' (benchmarks show ~20%
higher peak memory), drop the subjective 'most pythonic', document the batched
tuple shapes, and note that the nested layout is an unbind-only prototype whose
whole-tensor ops (.sum()/.shape) are unsupported.
Suggested-by: Fable
* fix: reject hybrid sparse tensors and gate nested up front
A 3-D hybrid sparse tensor (2 sparse + 1 dense dim) has ndim==3, so it slipped
past the rank check and was misrouted as batched, then crashed on index
unpacking. Add a dense_dim() != 0 guard to both public entry points, matching
the repo's existing dense-dim validation in utils (ValueError, 'zero dense
dimensions'). Also hoist the nested PyTorch>=2.4 version gate ahead of the
reduction so it no longer pays the full forward cost before raising.
Suggested-by: Fable
* test: cover batched layouts, wide matrices, and CSC
Close the periphery gaps the review flagged: batched padded/nested now assert
values (not just shapes), so a col/row plane swap in the batched padded assembly
can't pass; a transposed wide (4x5) case exercises the row-side -inf padding that
the tall fixtures never hit; and a forward-only CSC test reaches the free-col_nnz
path that the [coo, csr] fixture set never covered.
Suggested-by: Fable
* test: add batched grad parity, duplicate-COO, and raises matchers
Cover the remaining robustness gaps: a batched gradient-parity case (the
_bidir_batched backward is a distinct path, previously only probed manually); an
uncoalesced COO with repeated coordinates (duplicates must sum before exp, so a
refactor to _values()/_indices() that double-counts would fail); and match=
patterns on the two ValueError raises so an unrelated ValueError can't satisfy
them.
Suggested-by: Fable
* bench: make index-dtype labels truthful and fail loudly
COO always stores int64 indices, so an int32 request was silently promoted and
every 'int32' COO row (and the SuiteSparse CSR rows built via to_sparse_csr)
duplicated the int64 measurement under a wrong label. Skip COO+int32 loudly,
rebuild SuiteSparse CSR with the requested index dtype, and assert the built
dtype matches the label. Also hoist matrix generation out of the timing try so a
generation failure surfaces instead of being misattributed to the op.
Suggested-by: Fable
* docs: clarify n_groups is shared across batch slices
_scatter_logsumexp's output is a rectangular (*batch, n_groups), so n_groups
is one value for every slice. Document that a caller whose slices need
different group counts passes the max and pads — the surplus groups come back
-inf and can be sliced off, which is the contract the bidirectional path
relies on with G = max(nrows, ncols).
Addresses tvercaut's review comment on PR #89.
* docs: note col_lse/row_lse are views on the padded buffer
_bidir_2d returns the scatter's native output buffer alongside two basic-slice
views into it, so the three returns cost one allocation between them rather
than three.
Addresses tvercaut's review comment on PR #89.
* perf: only pay the padded transpose when that layout is requested
_bidir_batched returned padded.permute(1, 0, 2).contiguous(), materialising a
full (b, 2, G) copy on every batched call — including output_layout="tuple" and
"nested", which discard it untouched.
It now returns the scatter's native (2, b, G) buffer with col_lse/row_lse as
views into it, matching _bidir_2d, and the public function does the transpose
only in the "padded" branch. Public behaviour is unchanged: "padded" still
returns a contiguous (b, 2, G).
The copy cannot be avoided for "padded" itself — the scatter's batch axis
carries the two directions and the slice is folded into the group index, so the
native layout is inherently (2, b*G).
Measured on a (64, 8192, 8192) batch with 500k nnz: tuple-path peak CUDA memory
drops 70.8 -> 67.2 MB, the size of the discarded buffer.
Addresses tvercaut's review comment on PR #89.
* docs: note why _logsumexp_batched folds instead of using the batch axis
_scatter_logsumexp's batch axis needs a rectangular (*batch, nnz), but the
slices have ragged nnz. Padding them to (b, max_nnz) is correct but measured
2.6x slower at 1.4x the memory with evenly-filled slices, and degrades sharply
with skew (one heavy slice gave a 24x padding factor and 27x the time).
Addresses tvercaut's review comment on PR #89.
* docs: say where _logsumexp_batched's unequal-nnz slices come from
The comment said padding "the ragged slices" was slower without saying why the
slices are ragged, three lines under a docstring stating that batched CSR/CSC
require equal nnz per slice. The two read as contradictory.
Batched COO is the layout that permits unequal nnz — one global (3, nnz) index
matrix, so slices can hold any counts — and _logsumexp_batched coalesces every
input to COO, so that case is reachable. Name the layout in the comment.
Addresses tvercaut's review comment on PR #89.
* refactor: make the padded layout direction-first at both ranks
output_layout="padded" returned (2, G) unbatched but (batch, 2, G) batched, so
out[0] was col_lse in the first case and the first batch slice in the second.
The same argument meant different things depending on input rank.
Return the scatter's native (2, batch, G) buffer instead, and document it. The
direction now leads at both ranks: out[0] and out[1] are the column and row
reductions whatever the input. This is a breaking change to the "padded" shape.
Serving the old shape required a permute + contiguous, so this also drops a copy
and every layout is now a view into the one scatter allocation. That is worth
1.6-3.7% of the call (larger G, fewer nnz favours it), and no peak memory: the
copy was transient and sat under the scatter's own high-water mark.
Addresses tvercaut's review comment on PR #89.
* docs: publish sparse_bidir_logsumexp and flag "padded" as experimental
sparse_bidir_logsumexp was exported in __all__ but never added to
docs/source/api/core.rst, so its docstring rendered nowhere.
The "padded" layout hands back _scatter_logsumexp's native output buffer
without a copy, so its axis order, G = max(rows, cols) group padding and
-inf fill are all artifacts of that buffer. Say so, and point callers who
need a stable layout at "tuple".
Addresses tvercaut's review comment on PR #89.
Suggested-by: tvercaut
* docs: align batching terminology and state the batched CSR/CSC constraint
The docstrings described inputs as "2-D" / "batched 3-D (batch, rows, cols)",
which is not the vocabulary the rest of the repo uses. Say unbatched [r, c] and
batched [b, r, c] throughout, and spell the output shapes with the same letters
((b, c), (2, b, G), G = max(r, c)).
Both functions advertise CSR/CSC support, but not that PyTorch requires every
slice of a batched compressed tensor to hold the same number of specified
elements — a ragged batch is COO-only, and the caller hits that at construction,
before reaching us. Say so where the layouts are listed.
Addresses theo-barfoot's review comments on PR #89.
Referred-by: Theo Barfoot <theo.barfoot@gmail.com>
* test: cover CSR and CSC, batched and unbatched, in both logsumexp suites
The docstrings claim COO/CSR/CSC support at both ranks, but CSC only had a
single forward-only smoke test, and every batched test hardcoded .to_sparse_coo()
— so batched CSR/CSC, which reach the reduction by a different route (converted,
as batched compressed tensors expose no per-slice index accessors), were never
exercised.
Add FWD_LAYOUTS (= SPARSE_LAYOUTS + CSC) and drive the forward tests off it,
dropping the one-off test_csc_layout it subsumes. SPARSE_LAYOUTS still backs the
gradient tests: PyTorch has no backward for CSC values.
Batched CSR/CSC cannot represent unequal nnz per slice — PyTorch raises "Expect
the same number of specified elements per batch" at construction — so the ragged
_make_batched_dense fixture is COO-only by necessity. Add
_make_batched_dense_equal_nnz (one sparsity mask shared across slices, still with
an all-zero row so empty segments are covered) as the widest input all three
layouts can express, and use it for the new batched-layout tests.
Also pin the convention behind the batched path: _nnz() is a whole-tensor count
for COO but a per-slice count for batched CSR/CSC, so batched code reading it as
a total is wrong for the compressed layouts — which is why the batched reduction
converts to COO before counting.
282 -> 510 tests, all green.
Addresses theo-barfoot's review comment on PR #89.
Referred-by: Theo Barfoot <theo.barfoot@gmail.com>1 parent 9be6d3b commit dbbf6e2
9 files changed
Lines changed: 1024 additions & 38 deletions
File tree
- docs/source/api
- torchsparsegradutils
- benchmarks
- tests
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
22 | 25 | | |
23 | 26 | | |
24 | 27 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
32 | 32 | | |
33 | 33 | | |
34 | 34 | | |
| 35 | + | |
| 36 | + | |
35 | 37 | | |
36 | 38 | | |
37 | 39 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | | - | |
| 2 | + | |
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
| |||
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| 15 | + | |
15 | 16 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
| 24 | + | |
| 25 | + | |
24 | 26 | | |
25 | 27 | | |
26 | 28 | | |
| |||
Lines changed: 191 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
0 commit comments