Skip to content

[skip-ci] Fix groupby binary ops benchmarks #7603

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Mar 16, 2023
Merged

[skip-ci] Fix groupby binary ops benchmarks #7603

merged 5 commits into from
Mar 16, 2023

Conversation

dcherian
Copy link
Contributor

No description provided.

@dcherian dcherian added the run-benchmark Run the ASV benchmark workflow label Mar 10, 2023
@dcherian dcherian requested a review from Illviljan March 10, 2023 04:19
@Illviljan
Copy link
Contributor

Ahh, there are a few errors in the benchmarks. Are those fixed with your other PRs?

Error: [ 88.75%] ··· ...by.ResampleDask.peakmem_groupby_binary_op_2d             failed
Error: [ 88.75%] ···· Traceback (most recent call last):
                 File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 1293, in main_run_server
                   main_run(run_args)
                 File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 1167, in main_run
                   result = benchmark.do_run()
                 File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 573, in do_run
                   return self.run(*self._current_params)
                 File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 842, in run
                   self.func(*param)
                 File "/home/runner/work/xarray/xarray/asv_bench/benchmarks/groupby.py", line 136, in peakmem_groupby_binary_op_2d
                   self.ds2d.resample(time="48H") - self.ds2d_mean
                 File "/home/runner/work/xarray/xarray/asv_bench/.asv/env/e3a5540da3d30da735a9fb168f264be6/lib/python3.10/site-packages/xarray/core/_typed_ops.py", line 589, in __sub__
                   return self._binary_op(other, operator.sub)
                 File "/home/runner/work/xarray/xarray/asv_bench/.asv/env/e3a5540da3d30da735a9fb168f264be6/lib/python3.10/site-packages/xarray/core/groupby.py", line 601, in _binary_op
                   raise ValueError(
               ValueError: incompatible dimensions for a grouped binary operation: the group variable '__resample_dim__' is not a dimension on the other argument
               asv: benchmark failed (exit status 1)

@dcherian
Copy link
Contributor Author

No this is a real failure with no test coverage :(. I guess not many people do resampling binary ops?

@dcherian
Copy link
Contributor Author

Hmmm not sure why the resample binary ops benchmarks still fail.

@Illviljan
Copy link
Contributor 

 Error: [ 89.17%] ··· groupby.ResampleDask.time_binary_op_1d                      failed
Error: [ 89.17%] ···· Traceback (most recent call last):
                 File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 1293, in main_run_server
                   main_run(run_args)
                 File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 1167, in main_run
                   result = benchmark.do_run()
                 File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 573, in do_run
                   return self.run(*self._current_params)
                 File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 669, in run
                   samples, number = self.benchmark_timing(timer, min_repeat, max_repeat,
                 File "/home/runner/micromamba-root/envs/xarray-tests/lib/python3.10/site-packages/asv/benchmark.py", line 705, in benchmark_timing
                   timing = timer.timeit(number)
                 File "/home/runner/work/xarray/xarray/asv_bench/.asv/env/e3a5540da3d30da735a9fb168f264be6/lib/python3.10/timeit.py", line 178, in timeit
                   timing = self.inner(it, self.timer)
                 File "<timeit-src>", line 6, in inner
                 File "/home/runner/work/xarray/xarray/asv_bench/benchmarks/groupby.py", line 127, in time_binary_op_1d
                   raise NotImplementedError
               NotImplementedError
               asv: benchmark failed (exit status 1)

Comparing to

xarray/asv_bench/benchmarks/__init__.py

Lines 72 to 74 in 5043223

"""
if os.environ.get("ASV_SKIP_SLOW", "0") == "1":
raise NotImplementedError("Skipping this test...")

It has to be initialized? Try adding a short description with why it's not implemented/link to issue.

@dcherian dcherian merged commit e7b4930 into pydata:main Mar 16, 2023
@dcherian dcherian deleted the fix-groupby-binary-op-bench branch March 16, 2023 17:08
dcherian added a commit to dcherian/xarray that referenced this pull request Mar 26, 2023
@dcherian
Merge branch 'main' into kvikio
517f195 
* main: (40 commits)
  Faq pull request (According to pull request pydata#7604 & issue pydata#1285 (pydata#7638)
  add timeouts for tests (pydata#7657)
  Pull Request Labeler - Undo workaround sync-labels bug (pydata#7667)
  [pre-commit.ci] pre-commit autoupdate (pydata#7651)
  Allow all integer dtypes in `polyval` (pydata#7619)
  [skip-ci] dev whats-new (pydata#7660)
  Redo whats-new for 2023.03.0 (pydata#7659)
  Set copy=False when calling pd.Series (pydata#7642)
  Pin pandas < 2 (pydata#7650)
  Whats-new for release 2023.03.0 (pydata#7643)
  Bump pypa/gh-action-pypi-publish from 1.7.1 to 1.8.1 (pydata#7648)
  Use more descriptive link texts (pydata#7625)
  Fix missing 'dim' argument in _get_nan_block_lengths (pydata#7598)
  Fix `pcolormesh` with str coords (pydata#7612)
  [skip-ci] Fix groupby binary ops benchmarks (pydata#7603)
  Remove incomplete sentence in IO docs (pydata#7631)
  Allow indexing unindexed dimensions using dask arrays (pydata#5873)
  Bump pypa/gh-action-pypi-publish from 1.6.4 to 1.7.1 (pydata#7618)
  [pre-commit.ci] pre-commit autoupdate (pydata#7620)
  add a test for scatter colorbar extend (pydata#7616)
  ...
dcherian added a commit to kmsquire/xarray that referenced this pull request Mar 29, 2023
@dcherian
Merge remote-tracking branch 'upstream/main' into feature/rolling-pad
28a4ad2 
* upstream/main: (716 commits)
  Faq pull request (According to pull request pydata#7604 & issue pydata#1285 (pydata#7638)
  add timeouts for tests (pydata#7657)
  Pull Request Labeler - Undo workaround sync-labels bug (pydata#7667)
  [pre-commit.ci] pre-commit autoupdate (pydata#7651)
  Allow all integer dtypes in `polyval` (pydata#7619)
  [skip-ci] dev whats-new (pydata#7660)
  Redo whats-new for 2023.03.0 (pydata#7659)
  Set copy=False when calling pd.Series (pydata#7642)
  Pin pandas < 2 (pydata#7650)
  Whats-new for release 2023.03.0 (pydata#7643)
  Bump pypa/gh-action-pypi-publish from 1.7.1 to 1.8.1 (pydata#7648)
  Use more descriptive link texts (pydata#7625)
  Fix missing 'dim' argument in _get_nan_block_lengths (pydata#7598)
  Fix `pcolormesh` with str coords (pydata#7612)
  [skip-ci] Fix groupby binary ops benchmarks (pydata#7603)
  Remove incomplete sentence in IO docs (pydata#7631)
  Allow indexing unindexed dimensions using dask arrays (pydata#5873)
  Bump pypa/gh-action-pypi-publish from 1.6.4 to 1.7.1 (pydata#7618)
  [pre-commit.ci] pre-commit autoupdate (pydata#7620)
  add a test for scatter colorbar extend (pydata#7616)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Illviljan Illviljan Illviljan approved these changes

Assignees
No one assigned
Labels
run-benchmark Run the ASV benchmark workflow topic-performance
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dcherian @Illviljan