Use a static grid, asymmetric region, and xarray.testing for raster tests #1684

maxrjones · 2021-12-22T19:09:40Z

This issue was originally for discussing improvements to the PyGMT test suite and is now dedicated to tracking specific improvements:

Use xarray.testing rather than pygmt.grdinfo
Use static_earth_relief rather than earth_relief
Use asymmetric regions to facilitate identification of lat/lon errors

If anyone wants to help, please edit this comment to add your username next to the module or post a comment below to avoid redundant work.

The text was updated successfully, but these errors were encountered:

maxrjones · 2021-12-22T19:49:01Z

Thanks @willschlitzer for getting started on these tests! As @weiji14 mentioned in #593 (comment), we may want to work to address the likely failures due to upcoming @earth_relief failures during this process of migrating away from grdinfo. I have a few questions/comments regarding the tests:

Should we continue using the @earth_relief remote datasets for grid processing/plotting tests at all? We could instead create a xarray.DataArray with a subset of the V2.1 SRTM15+ data using a new helper function (e.g., pygmt.helpers.testing.static_earth_relief(). Probably no one wants to write out the data for 360 x 180 grid, so this would require determining whether any of the tests using global grids could use a smaller subset. The tests that use @earth_relief are listed below for context. For small grids (10x10), it is ~5% quicker to create a grid than use the remote data and could mean less maintenance work for future SRTM15+ updates.
I think we should move away from regions where the latitudes and longitudes could be switched without notice (e.g., region = [-5, 5, -5, 5]
We may need to adjust the rtol on assert_allclose for some tests - I'll post more on this later but accidentally opened this issue before working out the precision factors.

Processing/plotting tests using `load_earth_relief`:

Global pixel registration

test_grdcut.py - load_earth_relief(registration="pixel")
test_grd2cpt.py - load_earth_relief()
test_grdfilter.py - load_earth_relief(registration="pixel”)

Global gridline registration

test_grdimage.py - load_earth_relief(registration="gridline")
test_grdinfo.py - load_earth_relief(registration="gridline")
test_grdcontour.py - load_earth_relief(registration="gridline”)
test_makecpt.py - load_earth_relief(registration="gridline”)

Symmetric subset

test_grdgradient.py - load_earth_relief(resolution="01d", region=[-5, 5, -5, 5])
test_grdproject.py - load_earth_relief(resolution="01d", region=[-5, 5, -5, 5])
test_grdclip.py - load_earth_relief(resolution="01d", region=[-5, 5, -5, 5]
test_grdsample.py - load_earth_relief(resolution="01d", region=[-5, 5, -5, 5], registration=“pixel”)

Other subset

test_grd2xyz.py - load_earth_relief(resolution="01d", region=[-1, 1, 3, 5]
test_grdtrack.py - load_earth_relief(registration="gridline").sel(lat=slice(-49, -42), lon=slice(-118, -107))
test_grdvolume.py - load_earth_relief(resolution="01d", region=[-100, -95, 34, 39])
test_grdfill.py - load_earth_relief(registration="pixel", region=[125, 130, -25, -20])

maxrjones · 2021-12-22T20:15:44Z

These are the tests that can be expected to fail after the @earth_relief update to V2.3 (n=45):

FAILED ../pygmt/clib/session.py::pygmt.clib.session.Session
FAILED ../pygmt/clib/session.py::pygmt.clib.session.Session.virtualfile_from_grid
FAILED ../pygmt/tests/test_datasets_earth_relief.py::test_earth_relief_01d
FAILED ../pygmt/tests/test_datasets_earth_relief.py::test_earth_relief_01d_with_region
FAILED ../pygmt/tests/test_datasets_earth_relief.py::test_earth_relief_30m
FAILED ../pygmt/tests/test_grd2cpt.py::test_grd2cpt
FAILED ../pygmt/tests/test_grdclip.py::test_grdclip_outgrid
FAILED ../pygmt/tests/test_grdclip.py::test_grdclip_no_outgrid
FAILED ../pygmt/tests/test_grdcontour.py::test_grdcontour
FAILED ../pygmt/tests/test_grdcontour.py::test_grdcontour_labels
FAILED ../pygmt/tests/test_grdcontour.py::test_grdcontour_slice
FAILED ../pygmt/tests/test_grdcontour.py::test_grdcontour_file
FAILED ../pygmt/tests/test_grdcontour.py::test_grdcontour_interval_file_full_opts
FAILED ../pygmt/tests/test_grdcut.py::test_grdcut_file_in_file_out
FAILED ../pygmt/tests/test_grdcut.py::test_grdcut_file_in_dataarray_out
FAILED ../pygmt/tests/test_grdcut.py::test_grdcut_dataarray_in_file_out
FAILED ../pygmt/tests/test_grdcut.py::test_grdcut_dataarray_in_dataarray_out
FAILED ../pygmt/tests/test_grdfill.py::test_grdfill_file_out
FAILED ../pygmt/tests/test_grdfilter.py::test_grdfilter_dataarray_in_dataarray_out
FAILED ../pygmt/tests/test_grdfilter.py::test_grdfilter_dataarray_in_file_out
FAILED ../pygmt/tests/test_grdfilter.py::test_grdfilter_file_in_dataarray_out
FAILED ../pygmt/tests/test_grdfilter.py::test_grdfilter_file_in_file_out
FAILED ../pygmt/tests/test_grdgradient.py::test_grdgradient_outgrid
FAILED ../pygmt/tests/test_grdgradient.py::test_grdgradient_no_outgrid
FAILED ../pygmt/tests/test_grdinfo.py::test_grdinfo
FAILED ../pygmt/tests/test_grdinfo.py::test_grdinfo_file
FAILED ../pygmt/tests/test_grdinfo.py::test_grdinfo_region
FAILED ../pygmt/tests/test_grdproject.py::test_grdproject_file_out
FAILED ../pygmt/tests/test_grdproject.py::test_grdproject_no_outgrid
FAILED ../pygmt/tests/test_grdtrack.py::test_grdtrack_input_dataframe_and_dataarray
FAILED ../pygmt/tests/test_grdtrack.py::test_grdtrack_input_csvfile_and_dataarray
FAILED ../pygmt/tests/test_grdtrack.py::test_grdtrack_input_dataframe_and_ncfile
FAILED ../pygmt/tests/test_grdtrack.py::test_grdtrack_input_csvfile_and_ncfile
FAILED ../pygmt/tests/test_grdtrack.py::test_grdtrack_without_outfile_setting
FAILED ../pygmt/tests/test_grdview.py::test_grdview_with_perspective_and_zscale
FAILED ../pygmt/tests/test_grdview.py::test_grdview_with_perspective_and_zsize
FAILED ../pygmt/tests/test_grdview.py::test_grdview_with_cmap_for_perspective_surface_plot
FAILED ../pygmt/tests/test_grdview.py::test_grdview_on_a_plane
FAILED ../pygmt/tests/test_grdview.py::test_grdview_on_a_plane_with_colored_frontal_facade
FAILED ../pygmt/tests/test_grdview.py::test_grdview_with_perspective_and_zaxis_frame
FAILED ../pygmt/tests/test_grdview.py::test_grdview_surface_plot_styled_with_contourpen
FAILED ../pygmt/tests/test_grdview.py::test_grdview_surface_mesh_plot_styled_with_meshpen
FAILED ../pygmt/tests/test_grdview.py::test_grdview_on_a_plane_styled_with_facadepen
FAILED ../pygmt/tests/test_makecpt.py::test_makecpt_truncated_zlow_zhigh
FAILED ../pygmt/tests/test_makecpt.py::test_makecpt_continuous

willschlitzer · 2021-12-24T06:56:21Z

I think this is a good idea. Having a standardized grid should make it easier to write tests, and will save us time in the future. If nothing else, we should move grids to be over-land, as I assume land data is less likely to change instead of ocean data.

willschlitzer · 2021-12-27T15:06:06Z

@meghanrjones Should we work to change the region arguments for the tests that have symmetric or global regions right now, or wait until the earth_relief update comes and we also (hopefully) move to a local xarray file?

maxrjones · 2021-12-27T19:32:39Z

I think it would be better to update the region at the same as migrating to a locally defined xarray.DataArray to reduce reviewer work. If we chose to use a function that creates a DataArray rather than the remote files, we do not need to wait for the earth_relief update since we can create the grid based on the contents of SRTM15+V2.1.

willschlitzer · 2022-02-02T15:27:56Z

Are we planning on using preset DataArrays rather than grids from load_earth_relief? Now that the new Earth relief files have been pushed, it seems like we should decide how this issue will be handled.

weiji14 · 2022-02-02T20:38:18Z

I quite like @meghanrjones's idea at #1684 (comment) to have a pygmt.helpers.testing.static_earth_relief() function which could be a near drop-in replacement for existing tests using pygmt.datasets.load_earth_relief(). Or at least some function in pygmt.helpers.testing which loads a standard grid we can re-use across all tests.

There are some unit tests that run on an actual file (e.g. in test_grdimage.py) rather than an xarray.DataArray grid, so I'm thinking whether it's worth caching an @earth_relief_01d_p_v2.3 file (and possibly the gridline-registered version) at https://github.com/GenericMappingTools/gmtserver-admin/tree/master/cache which pygmt.helpers.testing.static_earth_relief() would pull from. Alternatively, we could generate a purely synthetic grid, even though I feel that testing on semi-real data is better.

maxrjones · 2022-02-02T22:22:18Z

There are some unit tests that run on an actual file (e.g. in test_grdimage.py) rather than an xarray.DataArray grid, so I'm thinking whether it's worth caching an @earth_relief_01d_p_v2.3 file (and possibly the gridline-registered version) at https://github.com/GenericMappingTools/gmtserver-admin/tree/master/cache which pygmt.helpers.testing.static_earth_relief() would pull from. Alternatively, we could generate a purely synthetic grid, even though I feel that testing on semi-real data is better.

We could also use @tut_bathy.nc for some tests rather than @earth_relief_01d. It's a bit smaller (11 KB versus 114 KB) and wouldn't require a new cache file.

gmt grdinfo @tut_bathy.nc
gmt grdimage @tut_bathy.nc -B -png test

Title: ETOPO5 global topography
Command: grdreformat -fg bermuda.grd bermuda.nc=ns
Remark: /home/elepaio5/data/grids/etopo5.i2
Gridline node registration used [Geographic grid]
Grid file format: ns = GMT netCDF format (16-bit integer), CF-1.7
x_min: -66 x_max: -60 x_inc: 0.0833333333333 (5 min) name: Longitude n_columns: 73
y_min: 30 y_max: 35 y_inc: 0.0833333333333 (5 min) name: Latitude n_rows: 61
v_min: -5475 v_max: -89 name: Topography [m]
scale_factor: 1 add_offset: 0
format: classic
Default CPT:

willschlitzer · 2022-02-03T11:22:53Z

There are some unit tests that run on an actual file (e.g. in test_grdimage.py) rather than an xarray.DataArray grid, so I'm thinking whether it's worth caching an @earth_relief_01d_p_v2.3 file (and possibly the gridline-registered version) at https://github.com/GenericMappingTools/gmtserver-admin/tree/master/cache which pygmt.helpers.testing.static_earth_relief() would pull from. Alternatively, we could generate a purely synthetic grid, even though I feel that testing on semi-real data is better.

We could also use @tut_bathy.nc for some tests rather than @earth_relief_01d. It's a bit smaller (11 KB versus 114 KB) and wouldn't require a new cache file.

This still seems like it covers a pretty big area/has a lot of data; wouldn't it speed up our plotting tests more to cache a smaller grid file (something like 10 x10 degree squadre at 30 minute resolution)?

weiji14 · 2022-02-03T15:08:22Z

There are some unit tests that run on an actual file (e.g. in test_grdimage.py) rather than an xarray.DataArray grid, so I'm thinking whether it's worth caching an @earth_relief_01d_p_v2.3 file (and possibly the gridline-registered version) at https://github.com/GenericMappingTools/gmtserver-admin/tree/master/cache which pygmt.helpers.testing.static_earth_relief() would pull from. Alternatively, we could generate a purely synthetic grid, even though I feel that testing on semi-real data is better.

We could also use @tut_bathy.nc for some tests rather than @earth_relief_01d. It's a bit smaller (11 KB versus 114 KB) and wouldn't require a new cache file.

This still seems like it covers a pretty big area/has a lot of data; wouldn't it speed up our plotting tests more to cache a smaller grid file (something like 10 x10 degree squadre at 30 minute resolution)?

Ok, how about we make a pygmt.helpers.testing.static_relief() based on a subset of the @tut_bathy.nc? Further to Meghan's point, we probably want a rectangular sample grid (e.g. 4 x 6 pixels) so that we don't get accidental errors with longitude and latitude mixed up.

willschlitzer · 2022-02-04T08:23:33Z

There are some unit tests that run on an actual file (e.g. in test_grdimage.py) rather than an xarray.DataArray grid, so I'm thinking whether it's worth caching an @earth_relief_01d_p_v2.3 file (and possibly the gridline-registered version) at https://github.com/GenericMappingTools/gmtserver-admin/tree/master/cache which pygmt.helpers.testing.static_earth_relief() would pull from. Alternatively, we could generate a purely synthetic grid, even though I feel that testing on semi-real data is better.

We could also use @tut_bathy.nc for some tests rather than @earth_relief_01d. It's a bit smaller (11 KB versus 114 KB) and wouldn't require a new cache file.

This still seems like it covers a pretty big area/has a lot of data; wouldn't it speed up our plotting tests more to cache a smaller grid file (something like 10 x10 degree squadre at 30 minute resolution)?

Ok, how about we make a pygmt.helpers.testing.static_relief() based on a subset of the @tut_bathy.nc? Further to Meghan's point, we probably want a rectangular sample grid (e.g. 4 x 6 pixels) so that we don't get accidental errors with longitude and latitude mixed up.

That works for me.

maxrjones · 2022-02-04T22:38:04Z

I started working on this but it turns out that it's super annoying to create a static DataArray based on a grid with a 0.0833333333333 grid spacing because of the checks in place for irregular grids. Based on this trial, I would prefer to go back to @weiji14's suggestion to add a small file to the cache based on @earth_relief_01d_p V2.3. I could submit a PR to add a file based on gmt grdcut @earth_relief_01d_p -R-55/-47/-24/-10 -Gstatic_earth_relief.nc to the cache. The output from this command is 1.6 KB with the following metadata:

static_earth_relief.nc: Title: Produced by grdcut
static_earth_relief.nc: Command: grdcut @earth_relief_01d_p -R-55/-47/-24/-10 -Gstatic_earth_relief.nc
static_earth_relief.nc: Remark: Reduced by Gaussian Cartesian filtering (111.2 km fullwidth) from SRTM15_V2.3.nc [Sandwell et al., 2022; https://doi.org/10.1029/2021EA002069]
static_earth_relief.nc: Pixel node registration used [Geographic grid]
static_earth_relief.nc: Grid file format: nf = GMT netCDF format (32-bit float), CF-1.7
static_earth_relief.nc: x_min: -55 x_max: -47 x_inc: 1 name: longitude n_columns: 8
static_earth_relief.nc: y_min: -24 y_max: -10 y_inc: 1 name: latitude n_rows: 14
static_earth_relief.nc: v_min: 190 v_max: 981 name: elevation (m)
static_earth_relief.nc: scale_factor: 1 add_offset: 0
static_earth_relief.nc: format: classic
static_earth_relief.nc: Default CPT: geo

willschlitzer · 2022-02-05T13:52:32Z

@meghanrjones This works for me! Once you submit that file to the cache I'll update the pull requests to not use a DataArray and instead used that cached grid file.

seisman · 2022-02-05T15:53:09Z

@willschlitzer is adding some inline examples in module docstrings (addressing #1686) which access the latest earth relief data by calling the load_earth_relief function. Although these inline examples are skipped in tests, some numbers in these examples will be outdated when new earth relief grids are released.

willschlitzer · 2022-02-06T07:50:57Z

@willschlitzer is adding some inline examples in module docstrings (addressing #1686) which access the latest earth relief data by calling the load_earth_relief function. Although these inline examples are skipped in tests, some numbers in these examples will be outdated when new earth relief grids are released.

Should I not be using load_earth_relief in the inline examples? I think that's the most accessible way for a new user to get an example grid to use to try a function.

willschlitzer · 2022-02-06T07:54:02Z

@meghanrjones Now that we have a standard file (which I'm assumed is called with @static_earth_relief), should we be calling the remote file directly or is the plan to add a function to call it?

maxrjones · 2022-02-06T17:10:50Z

@meghanrjones Now that we have a standard file (which I'm assumed is called with @static_earth_relief), should we be calling the remote file directly or is the plan to add a function to call it?

We can use @static_earth_relief for tests that run on an actual file, but I think we should add this file to pygmt.datasets.list_sample_data and pygmt.datasets.load_sample_data to return an xarray.DataArray for tests on DataArray input. I could submit a PR for this tomorrow AM, but if you or anyone else has time to add this before then please feel free to do so.

maxrjones · 2022-02-07T21:24:28Z

FYI I updated the checklist at the start of the issue to include all tests that currently use earth_relief except test_grdimage.py which requires a global dataset.

willschlitzer · 2022-02-13T09:16:15Z

Why wouldn't we use # doctest: +SKIP in the pygmt.clib.session example?

maxrjones · 2022-02-14T19:18:43Z

Why wouldn't we use # doctest: +SKIP in the pygmt.clib.session example?

The other modules have unit tests in the corresponding pygmt/tests/test_*.py, but that method does not have a unit test in test_clib.py

willschlitzer · 2022-02-20T10:43:54Z

I'm trying to rewrite the tests for grdtrack. I'm not able to use ocean_ride_points.txt with the static Earth relief grid input. Should we use a different input for grdtrack?

willschlitzer · 2022-02-23T18:26:53Z

FYI I updated the checklist at the start of the issue to include all tests that currently use earth_relief except test_grdimage.py which requires a global dataset.

Should we rewrite the grdimage tests? They have some of the longest test durations; I would assume they could be sped up significantly if we swapped out the grid to use static_earth_relief.

maxrjones · 2022-02-23T22:47:39Z

FYI I updated the checklist at the start of the issue to include all tests that currently use earth_relief except test_grdimage.py which requires a global dataset.

Should we rewrite the grdimage tests? They have some of the longest test durations; I would assume they could be sped up significantly if we swapped out the grid to use static_earth_relief.

It may be possible to use @static_earth_relief.nc for a few of the grdimage tests, but others are regression tests that require larger areas than in static_earth_relief. So it would require going through the issues referenced in each test to determine whether the bug could be reproduced with a smaller region.

maxrjones added the question Further information is requested label Dec 22, 2021

maxrjones mentioned this issue Dec 22, 2021

Wrap dimfilter #1492

Merged

5 tasks

maxrjones added maintenance Boring but important stuff for the core devs and removed question Further information is requested labels Feb 4, 2022

maxrjones changed the title ~~Test improvements during migration away from grdinfo?~~ Test improvements during migration away from grdinfo Feb 4, 2022

maxrjones mentioned this issue Feb 5, 2022

Add small grid based on earth_relief_01d V2.3 GenericMappingTools/gmtserver-admin#139

Merged

willschlitzer mentioned this issue Feb 6, 2022

Add load_static_earth_relief function for internal testing #1727

Merged

6 tasks

maxrjones changed the title ~~Test improvements during migration away from grdinfo~~ Use a static grid, asymmetric region, and xarray.testing for raster tests Feb 7, 2022

maxrjones mentioned this issue Feb 7, 2022

Update load_earth_relief tests for SRTM15+V2.3 #1728

Merged

6 tasks

This was referenced Feb 10, 2022

Use xarray.testing rather than pygmt.grdinfo in xyz2grd tests #1682

Merged

Add static_earth_relief to grd2cpt tests #1736

Merged

Add static_earth_relief to grd2xyz tests #1737

Merged

seisman added this to the 0.6.0 milestone Feb 16, 2022

willschlitzer mentioned this issue Feb 20, 2022

Change session.py to use static_earth_relief #1761

Merged

6 tasks

willschlitzer mentioned this issue Feb 22, 2022

Change test_grdtrack.py to use static_earth_relief #1762

Merged

6 tasks

seisman closed this as completed Mar 3, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use a static grid, asymmetric region, and xarray.testing for raster tests #1684

Use a static grid, asymmetric region, and xarray.testing for raster tests #1684

maxrjones commented Dec 22, 2021 •

edited by seisman

Loading

maxrjones commented Dec 22, 2021

maxrjones commented Dec 22, 2021

willschlitzer commented Dec 24, 2021

willschlitzer commented Dec 27, 2021

maxrjones commented Dec 27, 2021

willschlitzer commented Feb 2, 2022

weiji14 commented Feb 2, 2022 •

edited

Loading

maxrjones commented Feb 2, 2022

willschlitzer commented Feb 3, 2022

weiji14 commented Feb 3, 2022

willschlitzer commented Feb 4, 2022

maxrjones commented Feb 4, 2022

willschlitzer commented Feb 5, 2022

seisman commented Feb 5, 2022

willschlitzer commented Feb 6, 2022

willschlitzer commented Feb 6, 2022

maxrjones commented Feb 6, 2022

maxrjones commented Feb 7, 2022

willschlitzer commented Feb 13, 2022

maxrjones commented Feb 14, 2022

willschlitzer commented Feb 20, 2022

willschlitzer commented Feb 23, 2022

maxrjones commented Feb 23, 2022

Use a static grid, asymmetric region, and xarray.testing for raster tests #1684

Use a static grid, asymmetric region, and xarray.testing for raster tests #1684

Comments

maxrjones commented Dec 22, 2021 • edited by seisman Loading

maxrjones commented Dec 22, 2021

Processing/plotting tests using load_earth_relief:

maxrjones commented Dec 22, 2021

willschlitzer commented Dec 24, 2021

willschlitzer commented Dec 27, 2021

maxrjones commented Dec 27, 2021

willschlitzer commented Feb 2, 2022

weiji14 commented Feb 2, 2022 • edited Loading

maxrjones commented Feb 2, 2022

willschlitzer commented Feb 3, 2022

weiji14 commented Feb 3, 2022

willschlitzer commented Feb 4, 2022

maxrjones commented Feb 4, 2022

willschlitzer commented Feb 5, 2022

seisman commented Feb 5, 2022

willschlitzer commented Feb 6, 2022

willschlitzer commented Feb 6, 2022

maxrjones commented Feb 6, 2022

maxrjones commented Feb 7, 2022

willschlitzer commented Feb 13, 2022

maxrjones commented Feb 14, 2022

willschlitzer commented Feb 20, 2022

willschlitzer commented Feb 23, 2022

maxrjones commented Feb 23, 2022

maxrjones commented Dec 22, 2021 •

edited by seisman

Loading

Processing/plotting tests using `load_earth_relief`:

weiji14 commented Feb 2, 2022 •

edited

Loading