Skip to content

WIP: implement reductions as reductions - #172

Open
mbauman wants to merge 7 commits into
JuliaStats:masterfrom
mbauman:mb/reduce-reuse-recycle
Open

mbauman wants to merge 7 commits into
JuliaStats:masterfrom
mbauman:mb/reduce-reuse-recycle

Conversation

@mbauman

@mbauman mbauman commented Aug 27, 2024 •

Copy link
Copy Markdown
Contributor

Depends upon (and is a requirement for) Julia issue JuliaLang/julia#55318 JuliaLang/julia#58418.

Closes #7, closes #126, closes #160, closes #183.

mbauman added 2 commits May 14, 2025 17:43
Depends upon (and is a requirement for) Julia issue 55318
mbauman and others added 5 commits July 30, 2026 21:21
Brings the branch up to the upstream commit pinned by the JuliaLang/julia
mb/ReduceReuse♻ branch. Also restores upstream's explicit empty case in
the whole-array `_varm` method: computing it as `0/0` yields the
platform's default (negative) quiet NaN, while the canonical `NaN` the
tests compare against is positive.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The in-place `varm!` validated its `means` argument by passing every
dimension as the reduction region to `_checkm`, which demands a
singleton size along each axis. Any correctly-shaped means array — one
matching the output `R` — was therefore rejected with a
`DimensionMismatch`, and no test exercised `varm!` with array means.
Restore the intended check: `means` must have the same axes as `R`.

Also drop the special case that filled `R` with NaN whenever `A` had a
single element and `corrected=true`. The out-of-place dims path yields
`Inf` in that situation when the provided mean differs from the lone
element. Do what `_varm` does instead: when the correction leaves no
degrees of freedom, divide by zero, so a matching mean gives
0/0 = NaN and a differing mean gives Inf.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
The dims-variance kernel looked up each slice's mean through
`Broadcast._broadcast_getindex`, which bounds-checks the means array
for every element of `A`; that check accounted for most of the slowdown
against the old hand-rolled loops. Compute the broadcast index
explicitly and assert the access in-bounds.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
Detect the new reduction machinery positively, via
`isdefined(Base, :mapreduce_similar)`, instead of checking for a
removed internal. Collapse the duplicated cov/cor missing-propagation
branches into single `@test ... broken=` lines. Restore the exact
`=== NaN` assertion for the variance of an empty matrix, since the
whole-array path returns the canonical NaN again.

Add coverage that was missing while porting: `mean`/`var`/`std` over
general iterators (generators and `skipmissing`), and a check that
`var` over every dims combination of a 3-d array matches a naive
centralized-sum computation.

Assisted-By: Claude Fable 5 <noreply@anthropic.com>
This is the only place `CentralizedAbs2Fun{<:Extruded}` is constructed. Incidentally, the SparseArray extension already had its `centralize_sumabs2` function do this checking as well. It generally seems to fit better at this level (otherwise I would want to call it `unsafe_centralize_sumabs2`).

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

None yet

Projects

None yet

1 participant