PERF: GroupBy.any/all operate blockwise instead of column-wise #42841
Conversation
| const uint8_t[::1] mask, | ||
| def group_any_all(int8_t[:, ::1] out, | ||
| const int8_t[:, :] values, | ||
| const intp_t[::1] labels, |
There was a problem hiding this comment.
If we have a contiguity guarantee for labels here, would that also be the case for other groupby algos? Just wondering because I think the others don't specify labels contiguity, so might be able to gain small speedup by doing that.
There was a problem hiding this comment.
i think thats the case, yes. merits double-checking, especially since some builds are failing
pandas/_libs/groupby.pyx
Outdated
| @@ -443,26 +443,34 @@ def group_any_all(int8_t[::1] out, | |||
| else: | |||
| raise ValueError("'bool_func' must be either 'any' or 'all'!") | |||
|
|
|||
| if mask is not None and mask.shape[1] != 1: | |||
There was a problem hiding this comment.
i was wrong on this, will add suggested not None and consts in a separate PR
| def group_any_all(int8_t[:, ::1] out, | ||
| const int8_t[:, :] values, | ||
| const intp_t[::1] labels, | ||
| const uint8_t[:, :] mask, |
There was a problem hiding this comment.
See question below about if mask can be None, maybe it would be too verbose but could start using the not None addition to function definitions to make these explicit (for example const uint8_t[:, :] mask not None will reject None inputs
mzeitlin11
added
Algos
|
Why are we moving to 2d kernels here at all? we are moving away from these elsewhere (eg. recent refactor in rank to make the 2d one a wrapper around the 1d). I would rather NOT have 2d and 1d kernels for things. |
For perf, see benchmarks posted in OP.
my thought is that everything going through _get_cython_result here should be blockwise, which will both improve perf and let us de-duplicate with the existing groupby-blockwise code |
|
From #38992:
|
|
gentle ping, nearing completion on a follow-up that does the same perf-improvement for std |
|
can you rebase |
|
thanks, can you add a whatsnew in a followon |
The idea is to incrementally move all of the relevant functions to operate block-wise, at which point we can also de-duplicate _get_cythonized_result with _cython_agg_general.
Fleshes out the asv, which currently checks only single-column, to measure ncols=1, 2, 5, 10. We take a performance hit on the ncols==1 case and improve all the others (ex some noise)