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

Add bm_runner 'trialrun' subcommand. #5957

Merged
merged 8 commits into from
May 20, 2024

Conversation

pp-mo
Copy link
Member

@pp-mo pp-mo commented May 17, 2024

Closes #5944

Notes: I tried to make this as foolproof and self-contained as possible.
so it sets up DATA_GEN_PYTHON and ON_DEMAND_BENCHMARKS.

However you must still provide iris-test-data, via OVERRIDE_TEST_DATA_REPOSITORY (at least for most benchmarks).
If you don't, the resulting error is not that obvious (!) -- see below
You may also need to set BENCHMARK_DATA if you don't want to overfill your homespace or /tmp/persistent (here at MetOffice).
So, this is potentially still tricky to use, and even dangerous if you total your disk quota!

You may also provide a suitable environment, to prevent it creating one.
Omitting this does work, but can be slow as although we can build a suitable test env, we still don't have a reliable way of automatically caching + re-using them.
If the "Speed up environment prep" task currently on the board comes good, it might improve that case ?

Detail of error report due to missing test-data

$ nox -s benchmarks -- trialrun CubeCreation /tmp/persistent/newconda-envs/iris3/bin/python
nox > Running session benchmarks
nox > Re-using existing virtual environment at .nox/benchmarks.
nox > python -m pip install asv nox
nox > cd /tmp/persistent/git/iris/benchmarks
nox > python bm_runner.py trialrun CubeCreation /tmp/persistent/newconda-envs/iris3/bin/python
Couldn't load asv.plugins._mamba_helpers because
No module named 'libmambapy'
BM_RUNNER DEBUG: Using existing data generation environment.
BM_RUNNER DEBUG: Setting up ASV ...
BM_RUNNER DEBUG: asv machine --yes
Couldn't load asv.plugins._mamba_helpers because
No module named 'libmambapy'
I will now ask you some questions about this machine to identify it in the benchmarks.
  . . .
BM_RUNNER DEBUG: Setup complete.
BM_RUNNER DEBUG: asv run --bench CubeCreation --quick -e --environment existing:/tmp/persistent/newconda-envs/iris3/bin/python
Couldn't load asv.plugins._mamba_helpers because
No module named 'libmambapy'
· Ignoring ASV setting(s): `python`. Benchmark environment management is delegated to third party script(s).
· Discovering benchmarks
· Running 1 total benchmarks (1 commits * 1 environments * 1 benchmarks)
[ 0.00%] ·· Benchmarking existing-py_tmp_persistent_newconda-envs_iris3_bin_python
[50.00%] ··· cube.CubeCreation.time_create                                                                                                                                              failed
[50.00%] ··· =============== ============= ===========
             --                Cube creation strategy 
             --------------- -------------------------
              Cube has mesh   instantiate   construct 
             =============== ============= ===========
                  False          failed       failed  
                   True          failed       failed  
             =============== ============= ===========
             For parameters: False, 'instantiate'
             Traceback (most recent call last):

<<multiple tracebacks like this ...>>
              For parameters: True, 'construct'
              Traceback (most recent call last):
                File "/tmp/persistent/newconda-envs/iris3/lib/python3.11/site-packages/asv_runner/server.py", line 179, in _run_server
                  _run(run_args)
                File "/tmp/persistent/newconda-envs/iris3/lib/python3.11/site-packages/asv_runner/run.py", line 65, in _run
                  skip = benchmark.do_setup()
                         ^^^^^^^^^^^^^^^^^^^^
                File "/tmp/persistent/newconda-envs/iris3/lib/python3.11/site-packages/asv_runner/benchmarks/time.py", line 80, in do_setup
                  result = Benchmark.do_setup(self)
                           ^^^^^^^^^^^^^^^^^^^^^^^^
                File "/tmp/persistent/newconda-envs/iris3/lib/python3.11/site-packages/asv_runner/benchmarks/_base.py", line 632, in do_setup
                  setup(*self._current_params)
                File "/tmp/persistent/git/iris/benchmarks/benchmarks/cube.py", line 21, in setup
                  source_cube = realistic_4d_w_everything(w_mesh=w_mesh)
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                File "/tmp/persistent/git/iris/benchmarks/benchmarks/generate_data/stock.py", line 182, in realistic_4d_w_everything
                  _ = run_function_elsewhere(_external, w_mesh_=w_mesh, save_path_=str(save_path))
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                File "/tmp/persistent/git/iris/benchmarks/benchmarks/generate_data/__init__.py", line 95, in run_function_elsewhere
                  result = run(
                           ^^^^
                File "/tmp/persistent/newconda-envs/iris3/lib/python3.11/subprocess.py", line 571, in run
                  raise CalledProcessError(retcode, process.args,
              subprocess.CalledProcessError: Command '['/tmp/persistent/newconda-envs/iris3/bin/python', '-c', "def _external(w_mesh_: str, save_path_: str):\n    import iris\n    from iris.tests.stock import realistic_4d_w_everything\n\n    cube = realistic_4d_w_everything(w_mesh=bool(w_mesh_))\n    iris.save(cube, save_path_)\n\n_external(w_mesh_=True,save_path_='/data/users/itpp/benchmarks_testdata/realistic_4d_w_everything_True.nc')"]' returned non-zero exit status 1.
              asv: benchmark failed (exit status 1)

Traceback (most recent call last):
  File "/tmp/persistent/git/iris/benchmarks/bm_runner.py", line 636, in <module>
    main()
  File "/tmp/persistent/git/iris/benchmarks/bm_runner.py", line 632, in main
    parsed.func(parsed)
  File "/tmp/persistent/git/iris/benchmarks/bm_runner.py", line 596, in func
    _subprocess_runner(asv_command, asv=True)
  File "/tmp/persistent/git/iris/benchmarks/bm_runner.py", line 49, in _subprocess_runner
    return subprocess.run(args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/tmp/persistent/newconda-envs/tp_asvtst/lib/python3.12/subprocess.py", line 571, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['asv', 'run', '--bench', 'CubeCreation', '--quick', '-e', '--environment', 'existing:/tmp/persistent/newconda-envs/iris3/bin/python']' returned non-zero exit status 2.
nox > Command python bm_runner.py trialrun CubeCreation /tmp/persistent/newconda-envs/iris3/bin/python failed with exit code 1
nox > Session benchmarks failed.

@pp-mo pp-mo marked this pull request as ready for review May 17, 2024 09:51
@trexfeathers
Copy link
Contributor

trexfeathers commented May 17, 2024

Since OVERRIDE_TEST_DATA is mentioned as required in the README, perhaps we ought to have something in bm_runner.py that insists it is there. It would probably be unwelcome to automate the installation of iris-test-data, and I don't know how else we could automate the detection, but insisting on the environment variable would at least provide a quick failure and meaningful error message?

@trexfeathers
Copy link
Contributor

Since OVERRIDE_TEST_DATA is mentioned as required in the README, perhaps we ought to have something in bm_runner.py that insists it is there. It would probably be unwelcome to automate the installation of iris-test-data, and I don't know how else we could automate the detection, but insisting on the environment variable would at least provide a quick failure and meaningful error message?

Or maybe we SHOULD automate the installation, at the same time as Mule? It could be an identical approach?

Copy link
Contributor

@trexfeathers trexfeathers left a comment

Choose a reason for hiding this comment

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

I love this. Delivers exactly what is needed, especially useful as a guide to someone inexperienced with ASV.

benchmarks/bm_runner.py Outdated Show resolved Hide resolved
benchmarks/bm_runner.py Outdated Show resolved Hide resolved
benchmarks/bm_runner.py Outdated Show resolved Hide resolved
benchmarks/bm_runner.py Outdated Show resolved Hide resolved
benchmarks/bm_runner.py Show resolved Hide resolved
@pp-mo
Copy link
Member Author

pp-mo commented May 17, 2024

Sorry I just force-pushed because the pre-commit bot had added corrections.
I think I was using an env without pre-commit installed 🤦

Copy link

codecov bot commented May 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.78%. Comparing base (c956403) to head (b5d45a2).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5957   +/-   ##
=======================================
  Coverage   89.78%   89.78%           
=======================================
  Files          93       93           
  Lines       23007    23007           
  Branches     5017     5017           
=======================================
  Hits        20657    20657           
  Misses       1620     1620           
  Partials      730      730           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pp-mo
Copy link
Member Author

pp-mo commented May 17, 2024

Ok, latest commit I think addresses the review comments.
But I did make some more changes too, and notably this bit may not be quite correct in advance of #5958

benchmarks/bm_runner.py Outdated Show resolved Hide resolved
Copy link
Contributor

@trexfeathers trexfeathers left a comment

Choose a reason for hiding this comment

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

Couple of new points

benchmarks/bm_runner.py Outdated Show resolved Hide resolved
benchmarks/bm_runner.py Outdated Show resolved Hide resolved
@pp-mo
Copy link
Member Author

pp-mo commented May 20, 2024

Thanks @trexfeathers
Latest I hope addresses all the outstanding -- please re-review !

Copy link
Contributor

@trexfeathers trexfeathers left a comment

Choose a reason for hiding this comment

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

All seems to work, but the --help is slightly misleading so I have a suggestion

benchmarks/bm_runner.py Outdated Show resolved Hide resolved
benchmarks/bm_runner.py Outdated Show resolved Hide resolved
@trexfeathers trexfeathers merged commit 61eb74c into SciTools:main May 20, 2024
21 checks passed
@pp-mo
Copy link
Member Author

pp-mo commented May 20, 2024

Thanks for your patience, @trexfeathers !

stephenworsley added a commit to stephenworsley/iris that referenced this pull request Jun 10, 2024
* main: (759 commits)
  Bump scitools/workflows from 2024.05.1 to 2024.06.0 (SciTools#5986)
  [pre-commit.ci] pre-commit autoupdate (SciTools#5980)
  Updated environment lockfiles (SciTools#5983)
  Bump scitools/workflows from 2024.05.0 to 2024.05.1 (SciTools#5984)
  Make `slices_over` tests go faster (SciTools#5973)
  Updated environment lockfiles (SciTools#5979)
  Update lock files with associated fixes (SciTools#5953)
  List 25 slowest tests (SciTools#5969)
  used a note to highlight some text (SciTools#5971)
  Lazy `iris.cube.Cube.rolling_window` (SciTools#5795)
  Add memory benchmarks (SciTools#5960)
  Whatsnew for several benchmark developments. (SciTools#5961)
  Remove "on-demand" from some benchmarks (SciTools#5959)
  Add bm_runner 'trialrun' subcommand. (SciTools#5957)
  Automatically install iris-test-data for benchmark data generation (SciTools#5958)
  Added benchmarks for collapse and aggregate (SciTools#5954)
  Use tracemalloc for memory measurements. (SciTools#5948)
  Provide a Nox `benchmarks` session as the recommended entry point (SciTools#5951)
  [pre-commit.ci] pre-commit autoupdate (SciTools#5952)
  Remove unit benchmarks (SciTools#5949)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Benchmark runner convenience for "can I run X benchmark?"
2 participants