GenericMappingTools / pygmt

A Python interface for the Generic Mapping Tools.
https://www.pygmt.org
BSD 3-Clause "New" or "Revised" License
758 stars 220 forks source link

Benchmark performance of PyGMT functions #2910

Open weiji14 opened 10 months ago

weiji14 commented 10 months ago

Description of the desired feature

We are attempting some big refactoring steps in PyGMT to avoid the use of temporary intermediate files #2730, and will also be performing some updates to GMT 6.5.0 soon, so it would be good to track any performance improvements or regressions in terms of execution speed. Since #835, we have actually measured the execution time of the slowest tests (>0.2s) on CI for #584, but those execution times are not really tracked over time, and only hidden in the CI log files.

So, to better track performance over time, we are setting up a Continuous Benchmarking workflow in #2908. This is using pytest-codspeed, with the results logged to https://codspeed.io/GenericMappingTools/pygmt. The benchmarks is selective however, and will only be run on unit tests marked with @pytest.mark.benchmark.

Main question: Which tests do we decide to benchmark?

So which tests should we benchmark (and should we mark all those tests with @pytest.mark.benchmark in this PR or a follow-up one)?

I think we should focus on benchmarking low-level functions rather than modules' wrappers, since the low-level functions are heavily used in everywhere and most wrappers have very simple and similar code structures.

For example, most plotting modules have very simple code like the one in Figure.basemap

    kwargs = self._preprocess(**kwargs)                                         
    with Session() as lib:                                                      
        lib.call_module(module="basemap", args=build_arg_string(kwargs))

so we just need to benchmark one basemap test (e.g., test_basemap()) and don't need to benchmark other basemap's tests and other plotting methods (e.g., Figure.coast). Of course, there are few exceptions, for example, Figure.meca, Figure.plot, Figure.plot3d, Figure.text are among the complicated wrappers and should be benchmarked.

Similarly, for table-processing and grid-processing functions, benchmarking pygmt.select and pygmt.grdfill should be enough.

Originally posted by @seisman in https://github.com/GenericMappingTools/pygmt/issues/2908#issuecomment-1868514665

Other questions: When do we want to run the benchmarks? Should this be enabled on every Pull Request, or just Pull Requests that modify certain files perhaps? Edit: Answered in https://github.com/GenericMappingTools/pygmt/pull/2908#discussion_r1435894337

Are you willing to help implement and maintain this feature?

Yes

weiji14 commented 10 months ago

I think we should focus on benchmarking low-level functions rather than modules' wrappers, since the low-level functions are heavily used in everywhere and most wrappers have very simple and similar code structures.

Agree with benchmarking the low-level clib functions, but I have other thoughts about benchmarking the wrappers (see below).

so we just need to benchmark one basemap test (e.g., test_basemap()) and don't need to benchmark other basemap's tests and other plotting methods (e.g., Figure.coast). Of course, there are few exceptions, for example, Figure.meca, Figure.plot, Figure.plot3d, Figure.text are among the complicated wrappers and should be benchmarked.

Similarly, for table-processing and grid-processing functions, benchmarking pygmt.select and pygmt.grdfill should be enough.

Considering that we will be switching from GMT 6.4.0 to 6.5.0 soon, I almost think we should have at least one benchmark for each of the 60+ PyGMT modules listed in pygmt/src/__init__.py. That way, we can spot any potential performance regressions in GMT. The same could be said for any of the upstream libraries we depend on such as NumPy, Pandas, Xarray, GeoPandas, etc.

I assumed that the benchmarking would be slow in https://github.com/GenericMappingTools/pygmt/issues/2730#issuecomment-1863359297 since they might need to run a benchmark multiple times, but actually after reading https://codspeed.io/blog/pinpoint-performance-regressions-with-ci-integrated-differential-profiling, codspeed only runs each benchmark once!

The action will run the benchmarks on a "virtualized" CPU with Valgrind. Instead of measuring the execution time, it measures the number of CPU cycles and memory accesses. Each benchmark is only ran once and it is enough to get consistent data.

So it might be ok to have 60+ benchmarks running at once. Plus pytest-codspeed can be combined with pytest-xdist (see https://docs.codspeed.io/benchmarks/python#running-benchmarks-in-parallel), so we could potentially speed up the benchmarking further.

seisman commented 10 months ago

Considering that we will be switching from GMT 6.4.0 to 6.5.0 soon, I almost think we should have at least one benchmark for each of the 60+ PyGMT modules listed in pygmt/src/__init__.py. That way, we can spot any potential performance regressions in GMT. The same could be said for any of the upstream libraries we depend on such as NumPy, Pandas, Xarray, GeoPandas, etc.

Or maybe we should just benchmark all tests except those for catching exceptions.

weiji14 commented 10 months ago

Or maybe we should just benchmark all tests except those for catching exceptions.

Maybe not all all, but at least one (or two) for each PyGMT function for now?

seisman commented 10 months ago

I'm OK with that.

weiji14 commented 10 months ago

Ok, here's the list of test files:



I'm gonna work on the second half (from test_geopandas to test_xyz2grd) in #2911 for now.

weiji14 commented 10 months ago

Putting this here to remind ourselves to document guidelines for Continuous Benchmarking before closing this issue.

@weiji14 Do you want to add a section like "Continuous Benchmark" to this guide?

I did think about it when writing the docs in #2908, but wasn't sure what to include yet in terms of guidelines. Maybe after a few weeks/months when we've got some experience with benchmarking, then we can add some words in.

Originally posted by @weiji14 in https://github.com/GenericMappingTools/pygmt/issues/2916#issuecomment-1870015308

weiji14 commented 9 months ago

At commit https://github.com/GenericMappingTools/pygmt/pull/2908/commits/dedfa7af7250762f473a81928a609b04198d11b1 in #2908, and the follow-up patch in #2952, we set the benchmark workflow to only run on merge to main branch when '*.py' files or the '.github/workflows/benchmarks.yml' is modified. However, in https://github.com/GenericMappingTools/pygmt/pull/3040#issuecomment-1928545734, I triggered the benchmark runs, and the flame graphs are not showing up (there is a message like No baseline data is yet available. Once your main branch has generated a first performance report, you will see the list of commits and their performance impact here), possibly because that PR cannot compare to a baseline run on the main branch (https://github.com/GenericMappingTools/pygmt/commit/72131ae1c5158424236eae4ebcb718dfe7186420) which did not have a benchmark run.

Luckily, we can manually trigger a baseline benchmark run on the main branch! Steps are:

  1. Go to https://github.com/GenericMappingTools/pygmt/actions/workflows/benchmarks.yml
  2. Click on the 'Run Workflow' button on the right. There should be an option to trigger the workflow on the main branch (or any tag/ref): image
  3. Wait for the baseline benchmark run to finish (takes about 14min)
  4. Trigger the benchmark run on the PR branch by adding the run/benchmark tag.
  5. View the flame graphs at https://codspeed.io/GenericMappingTools/pygmt/branches/!