WIP: implement reductions as reductions - #172
Open
mbauman wants to merge 7 commits into
Open
Conversation
Depends upon (and is a requirement for) Julia issue 55318
mbauman
force-pushed
the
mb/reduce-reuse-recycle
branch
from
May 14, 2025 21:50
f31afb5 to
c674ab8
Compare
11 tasks
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`).
mbauman
force-pushed
the
mb/reduce-reuse-recycle
branch
from
August 6, 2026 22:10
d84e226 to
a7ef774
Compare
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.
Depends upon (and is a requirement for) Julia issue
JuliaLang/julia#55318JuliaLang/julia#58418.Closes #7, closes #126, closes #160, closes #183.