Skip to content

Fix sparse mapreduce for operators other than + - #796

Open
maleadt wants to merge 1 commit into
mainfrom
tb/sparse-mapreduce
Open

maleadt wants to merge 1 commit into
mainfrom
tb/sparse-mapreduce

Conversation

@maleadt

@maleadt maleadt commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

Sparse mapreduce had two bugs, both reproducible with JLArrays:

  • the row and column kernels started from op(init, init) with init = zero(eltype), so maximum of a row whose stored entries are all negative returned 0 (also for the full reduction, which reduced the per-row results with the same zero init);
  • implicit zeros were folded in as nzeros * f(0), which is only right for +: mapreduce(x -> x + 1, max, A) for a 3×3 matrix with two stored entries gave 3.0 instead of 1.0.

Each slice now reduces its stored values starting from the first one and folds in its implicit zeros afterwards with op itself (by repeated squaring, or once for idempotent operators such as max), which is what SparseArrays does. The full reduction reduces the stored values with the dense mapreduce and folds the zeros in on the host. An explicit init is combined with each result, empty slices follow Base (init, or the reduction of an empty collection), and dims=2 returns an m×1 matrix like Base instead of a vector.

Tests: all-negative matrices with and without implicit zeros (maximum/minimum over every dims), non-zero-preserving functions with *, max and min, init, an operator without a known neutral element, result shapes, and empty dimensions.

Stacked on #795, below the sparse redesign in #745; it can go into 11.x on its own.

@maleadt
maleadt added this pull request to stack #797 September 25, 2026 14:56
@maleadt
maleadt force-pushed the tb/sparse-mapreduce branch from 9d6c826 to 48bb64d Compare September 25, 2026 15:24
@maleadt maleadt added the arrays The array abstraction: broadcasting, mapreduce, indexing, views, generic linear algebra. label Oct 19, 2026
@maleadt
maleadt removed this pull request from stack #797 October 21, 2026 13:00
@maleadt
maleadt added this pull request to stack #832 October 21, 2026 13:00
@maleadt
maleadt force-pushed the tb/sparse-vector-broadcast branch from 243732e to 55a9018 Compare October 22, 2026 10:46
@maleadt
maleadt force-pushed the tb/sparse-mapreduce branch from 48bb64d to 05b2b7c Compare October 22, 2026 10:46
@maleadt
maleadt force-pushed the tb/sparse-vector-broadcast branch from 55a9018 to 6d1f474 Compare October 22, 2026 10:54
@maleadt
maleadt force-pushed the tb/sparse-mapreduce branch from 05b2b7c to f5a2181 Compare October 22, 2026 10:54
@maleadt
maleadt marked this pull request as ready for review October 23, 2026 08:11
The row and column reduction kernels started from op(init, init) with
init = zero(eltype), so maximum of a row whose stored entries are all
negative returned zero, and they folded the implicit zeros in as
nzeros * f(0), which is only right for +: mapreduce(x -> x + 1, max, A)
counted the implicit zeros instead of taking their maximum.

Reduce the stored values of each slice starting from the first one, and
fold in the implicit zeros afterwards with op itself (by repeated
squaring, or once for idempotent operators), as SparseArrays does. The
full reduction now reduces the stored values and folds the zeros in on
the host instead of reducing per-row results with a zero init. An
explicit init is combined with each result, empty slices follow Base,
and dims=2 returns an m×1 matrix like Base instead of a vector.
@maleadt
maleadt force-pushed the tb/sparse-mapreduce branch from f5a2181 to 725b64c Compare October 23, 2026 08:17
@maleadt
maleadt removed this pull request from stack #832 October 23, 2026 08:18
@maleadt
maleadt changed the base branch from tb/sparse-vector-broadcast to main October 23, 2026 08:18

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrays The array abstraction: broadcasting, mapreduce, indexing, views, generic linear algebra.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant