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

Simplify the tests for the greenspline module #6326

Open
maxrjones opened this issue Feb 9, 2022 · 5 comments
Open

Simplify the tests for the greenspline module #6326

maxrjones opened this issue Feb 9, 2022 · 5 comments
Assignees
Labels
maintenance Boring but important stuff for the core devs

Comments

@maxrjones
Copy link
Member

Two of the 10 slowest tests are for the greenspline module (gspline_2.sh and gspline_6.sh). However, a small fraction of the time for these two tests is actually spent in the greenspline module (~0.2s greenspline, ~21s surface, ~0.5s plotting + graphicsmagick time). Here are a couple ideas for simplifying these tests to take less time:

  1. Use ncdump or a similar tool to compare the output from greenspline to a reference grid file, removing the need for surface or plotting modules completely.
  2. Use either DVC or the GMT cache to store the output from gmt surface @Table_5_11.txt -R$R -I$D -Graws5.grd -T0.5 -N10000 -C0.0000001, such that the slow surface step does not take place in the greenspline tests.

I would prefer using option 1 for the daily tests of greenspline. Since the tests also serve to guarantee that the figures in Paul's 2009 paper are reproducible, perhaps we could have a separate set of tests for published figures that can be optionally run (similar to the API tests and the proposal for the supplement tests in #6324). Any thoughts @GenericMappingTools/gmt-maintainers?

@maxrjones maxrjones added feature request Request a new feature maintenance Boring but important stuff for the core devs labels Feb 9, 2022
@PaulWessel
Copy link
Member

I like the idea of storing the surface grid in DVC. A key benefit of having published paper script in the test is that decades after they were published we would expect GMT to still produce them as intended. These figures are (possibly) seen my more (or different) people and some may try to emulate those plots.

@joa-quim
Copy link
Member

joa-quim commented Feb 9, 2022

Prefer opt 1 too. And no need for an external tool. The output grid can be passed to grdinfo -C and do only numeric comparisons.

@maxrjones
Copy link
Member Author

I agree that we should still have a way to test whether the published script produces the same output, but I am less convinced that we should test that on every commit to the master branch. Even option 2 would not guarantee that the script works as published because breaking changes to surface could be missed. What if we were to use option 1 to simplify the daily greenspline tests and create a new 'published' directory in test that can be tested weekly in the CI using a CMake setting similar to DO_API_TESTS? A quick search suggests that these are the tests based on published GMT figures:

./gmtmex/WL_example_1.sh
./gmtmex/WL_example_2.sh
./gmtmex/WL_example_3.sh
./greenspline/gspline_1.sh
./greenspline/gspline_2.sh
./greenspline/gspline_3.sh
./greenspline/gspline_4.sh
./greenspline/gspline_5.sh
./pstext/caption.sh
./x2sys/x2sys_01.sh
./x2sys/x2sys_02.sh
./x2sys/x2sys_03.sh
./x2sys/x2sys_04.sh

@maxrjones
Copy link
Member Author

Regarding using grdinfo -C, yes it's best to avoid external tools and it's unlikely that the grid would change without affecting those statistics so that should work.

@PaulWessel
Copy link
Member

I am fine with a less frequent check on some scripts, but given the history of all tests (we end up braking something while fixing something else via whack-a-mole and the failing tests help us understand and fix the issue. That is the use of such plot scripts. GMT is a very visual-oriented toolset.

@maxrjones maxrjones self-assigned this Feb 9, 2022
@maxrjones maxrjones added this to To Do in GMT testing improvements via automation Mar 8, 2022
@maxrjones maxrjones removed the feature request Request a new feature label Mar 18, 2022
@maxrjones maxrjones added this to the 6.4.0 milestone Mar 23, 2022
@maxrjones maxrjones modified the milestones: 6.4.0, Future release May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Boring but important stuff for the core devs
Development

No branches or pull requests

3 participants