-
Notifications
You must be signed in to change notification settings - Fork 224
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
Setup Continuous Benchmarking workflow with pytest-codspeed #2908
Conversation
Measuring the execution speed of tests to track performance of PyGMT functions over time. Using pytest-codspeed, see https://docs.codspeed.io/benchmarks/python#running-the-benchmarks-in-your-ci. Decorated a couple of unit tests with @pytest.mark.benchmark to see if the benchmarking works.
To debug why import pygmt doesn't work.
Might need to launch into the correct shell first. Also running pip list to double check if dependencies are installed ok.
Seeing if it's possible to avoid using conda shell.
Do everything in the same shell, but point to the GMT installation in /home/runner/micromamba/envs/pygmt/lib/
Try to prevent `ERROR: Project file:///home/runner/work/pygmt/pygmt has a 'pyproject.toml' and its build backend is missing the 'build_editable' hook. Since it does not have a 'setup.py' nor a 'setup.cfg', it cannot be installed in editable mode. Consider using a build backend that supports PEP 660`.
Need newer setuptools greater than 64.
Action might not like the single quotes.
a19bdf2
to
31a7567
Compare
Should be /home/runner/micromamba/envs/pygmt/lib/, but don't know what's happening in the CodSpeedHQ/action shell.
Maybe best to use the bundled conda with GitHub Actions?
New place where GMT is installed by miniconda.
See if libgmt.so is in this folder.
Trying to workaround `Error loading GMT shared library at '/usr/share/miniconda/envs/pygmt/lib/libgmt.so'. /lib/x86_64-linux-gnu/libcrypto.so.3: version `OPENSSL_3.2.0' not found (required by /usr/share/miniconda/envs/pygmt/lib/././libssl.so.3)`
Missing conda-forge package for pytest-codspeed.
Instead of setting Python 3.12 explicitly.
Don't use `make test`, which seems to use the system python in `/usr/bin/python` rather than `/usr/share/miniconda/bin/python`.
Also split the PyGMT build step into a separate step, now that we're confident all the packages are installed using conda's python/pip.
Also tidy up some stray bits
Try to avoid collecting tests in the examples/ folder.
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
so we just need to benchmark one Similarly, for table-processing and grid-processing functions, benchmarking |
Ok, done at c92fcb2 and 7bf09b9
Probably not needed in the checklist, but I added a trigger at c8d1965 so that the benchmarks will be run when a release is published.
Let me open up a separate issue to discuss this, and also to track which unit tests we should benchmark. Edit: see #2910. |
Remove markers for test_blockmean_input_dataframe and test_grd2xyz.
The default Python used now should be the conda one instead of the system one.
Default `make test` requires the use of pytest-cov and pytest-doctestplus
Trigger the benchmark run when files in `pygmt/clib`, `pygmt/datasets`, `pygmt/helpers`, `pygmt/src` and `pygmt/*.py` are modified (i.e. except `pygmt/tests/**`), and also when .github/workflows/benchmarks.yml is modified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
The Tests workflow (https://github.com/GenericMappingTools/pygmt/actions/runs/7319763038/job/19937998567) now has the following warnings because
I believe we need to silence these warnings. |
Ok, opened #2912 for this. |
By the way, can you change the CodSpeed settings to only make PR comments when there is a performance improvement or regression (still don't have permissions)? This should reduce noise like #2907 (comment) when there's no noticeable change (we can still see the report in the 'Checks' tab if needed). Also, maybe reduce the regression threshold to 5%? |
Done. |
Description of proposed changes
Measuring the execution speed of tests to track performance of PyGMT functions over time.
Using
pytest-codspeed
to do the benchmarking, with the help of https://github.com/CodSpeedHQ/action. Decoratedtest_basemap
with@pytest.mark.benchmark
to see if the benchmarking works.Note: Running the benchmarks on Python 3.12 to enable flame graph generation, available with
pytest-codspeed>=2.0.0
- see https://docs.codspeed.io/features/trace-generation.References:
Relates to #2730 (comment), addresses #2910.
Reminders
make format
andmake check
to make sure the code follows the style guide.doc/api/index.rst
.Slash Commands
You can write slash commands (
/command
) in the first line of a comment to performspecific operations. Supported slash commands are:
/format
: automatically format and lint the code/test-gmt-dev
: run full tests on the latest GMT development version