Skip to content
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

Enable numbagg for reductions #8316

Merged
merged 9 commits into from
Oct 18, 2023
Merged

Enable numbagg for reductions #8316

merged 9 commits into from
Oct 18, 2023

Conversation

dcherian
Copy link
Contributor

@dcherian dcherian commented Oct 16, 2023

  • Tests added - check bottleneck tests
  • User visible changes (including notable bug fixes) are documented in whats-new.rst

@dcherian dcherian requested a review from max-sixty October 16, 2023 04:46
@dcherian dcherian added the run-benchmark Run the ASV benchmark workflow label Oct 16, 2023
Copy link
Collaborator

@max-sixty max-sixty left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks @dcherian !

Are there any other functions that would be helpful?

Do we need to capitulate on the ddof?

except ImportError:
# use numpy methods instead
numbagg = np
_NUMBAGG_AVAILABLE = False
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI there's a version of this at

has_numbagg = numbagg.__version__
, which has the advantage that it contains the version, which we likely want to filter on...

(though I'm not sure my approach is that good! Open to feedback...)

@dcherian dcherian marked this pull request as ready for review October 16, 2023 14:30
@dcherian dcherian marked this pull request as draft October 16, 2023 15:09
@andersy005 andersy005 added plan to merge Final call for comments and removed plan to merge Final call for comments labels Oct 16, 2023
@dcherian
Copy link
Contributor Author

Waiting on numbagg/numbagg#162 to be released so I can add a version check and only defer to numbagg when ddof=1 (instead of 0 currently)

@max-sixty
Copy link
Collaborator

Waiting on numbagg/numbagg#162 to be released so I can add a version check and only defer to numbagg when ddof=1 (instead of 0 currently)

Done!

@dcherian
Copy link
Contributor Author

Should work whenever the CI environment updates.

@dcherian dcherian marked this pull request as ready for review October 17, 2023 18:14
@dcherian dcherian added the plan to merge Final call for comments label Oct 17, 2023
max-sixty added a commit to max-sixty/xarray that referenced this pull request Oct 17, 2023
Uses the approach in pydata#8316, a bit nicer. Only internal.
max-sixty added a commit to max-sixty/xarray that referenced this pull request Oct 17, 2023
Uses the approach in pydata#8316, a bit nicer. Only internal.
@andersy005 andersy005 enabled auto-merge (squash) October 17, 2023 19:21
@andersy005 andersy005 disabled auto-merge October 18, 2023 10:39
@andersy005 andersy005 merged commit ae41d82 into pydata:main Oct 18, 2023
25 of 26 checks passed
@dcherian dcherian deleted the numbagg branch October 18, 2023 14:54
max-sixty added a commit that referenced this pull request Oct 19, 2023
* internal: Improve version handling for numbagg

Uses the approach in #8316, a bit nicer. Only internal.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Update xarray/core/rolling_exp.py

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plan to merge Final call for comments run-benchmark Run the ASV benchmark workflow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants