-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: implement sum
, count
, and count_nonzero
reductions
#347
feat: implement sum
, count
, and count_nonzero
reductions
#347
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #347 +/- ##
==========================================
- Coverage 83.01% 82.91% -0.10%
==========================================
Files 96 96
Lines 11372 11435 +63
==========================================
+ Hits 9440 9481 +41
- Misses 1932 1954 +22
☔ View full report in Codecov by Sentry. |
Instead they should just be array-function overloads
…o agoose77/feat-sum-reducer # Conflicts: # src/vector/backends/numpy.py
…o agoose77/feat-sum-reducer
Here's a rewrite that implements the reductions for each array backend. I need to cleanup the type hints, but it's mostly there. |
We don't need this now we exclusively use the NEP-18 dispatcher
This should also close #92 and maybe iris-hep/func_adl_uproot#98 🎉 PS: I had a very dirty somewhat working implementation for |
Oops, I missed this! Well, good to know it's needed! |
NB, this PR took a journey following feedback from Jim. In its current (final!) form, we don't worry with coordinate-systems; everything is moved into Cartesian, and remains in Cartesian. |
Some (many?) of the ufunc-like functions do, too. This is not unique to reducers. |
This is ready to go! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is ready to go now because Awkward 2.2.3 is out. (The tests would have failed if they couldn't reach it.)
I did another scan through the diffs, and I'm reminded that this includes some bug-fixes as well as the new feature, but I guess that's alright. (There's assignment of __numba_typer__
and __numba_lower__
into Vector's behavior
instead of ak.behavior
, and also type hint corrections on allclose
—booleans, not floats—that I hadn't noticed before.)
I'll merge it.
@jpivarski @Saransh-cpp this is a WIP initial stab at supporting
sum
,count
, andcount_nonzero
reducers in vector across both NumPy and Awkward.1My main query here is around API design; I note that @jpivarski and @henryiii designed the initial prototype, so you'll both likely have stronger feelings on how this deviates from the intended design. In particular, is it reasonable to fit reducers (in this case, only
sum
) into the compute model that is already established? From our type hints here, we don't explicitly encode any expectation that the vectors in compute functions are array like (or indeed satisfy any type checks), so I think this is fine.Awkward support depends upon scikit-hep/awkward#2458.
Footnotes
count
is Awkward specific, however. ↩