Fix test against collect #55
Conversation
Also removes dependency on saved binary files.
Required for test_against_collect
This is needed for BOUT++'s 'collect' function to work correctly, as for BOUT_VERSION<3.5 it was necessary to ignore the last point in the z-grid, so collect needs to be able to check the BOUT_VERSION.
These scalar variables change from processor to processor, so are dropped when loading a dataset and cannot be checked.
In some tests, these need to be dropped in addition to METADATA_VARS before comparing generated and loaded datasets.
Returning 0 for an upper limit causes a bug, as the data is sliced with slice(lower, upper), where upper=-<number of guard cells>, but -0=0 so the slice would remove all the data.
Can then ignore it safely in the test suite.
Many tests produce a warning from xarray like:
RuntimeWarning: deallocating CachingFileManager(<class 'netCDF4._netCDF4.Dataset'>, '/tmp/pytest-of-***/<path-to-file>', mode='r', kwargs={'clobber': True, 'diskless': False, 'persist': False, 'format': 'NETCDF4'}), but file is not already closed. This may indicate a bug.
Ignore these warnings, as they do not tell us anything useful about
xBOUT.
|
Hello @johnomotani! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2019-12-03 22:30:25 UTC |
In Python-3.6 the mayavi install fails if vtk is not available. mayavi is a dependency of boutdata, needed for test_against_collect. As a workaround, pip-install vtk before pip-installing boutdata.
May be needed to provide some variables that are not saved by default to BOUT++'s output files, e.g. psixy, Rxy, Zxy.
Need to add hthe before getting toroidal coordinates, as dimension names are changed only in the Dataset, not in the _grid member variable. 'r' coordinate is created as 1d, so selecting 'theta=0' part is an error.
| [pytest] | ||
| filterwarnings = | ||
| ignore::xbout.warning.xBOUTWarning | ||
| ignore:deallocating CachingFileManager.*, but file is not already closed. This may indicate a bug\.:RuntimeWarning |
There was a problem hiding this comment.
We should actually fix whatever is causing this warning...
There was a problem hiding this comment.
I agree in principle, but this warning also seems to be thrown in xarray's test suite (pydata/xarray#3266), so I guess it's an xarray issue. The warnings seem to be thrown during garbage-collection (I think I saw a comment somewhere that it might be related to some sort of reference-cycle, but I forget where now), and I didn't want to go chasing into the guts of xarray.
Doing this prevented the dataset being saved to netCDF file. Also use _open_grid instead of open_boutdataset to open the grid file so that the grid dataset does not have metadata added to the DataArray variables. These variables are added as coordinates and become members of the DataArrays representing simulation variables; if they have a metadata dict, it is not possible to save them to netCDF.
...when hthe is loaded from the grid file. Variables from the grid file do not have all the correct metadata, so cause errors when trying to save to netCDF.
Depedency has been made optional by an update to boututils.
Depedency has been made optional by an update to boututils.
Codecov Report
@@ Coverage Diff @@
## master #55 +/- ##
=========================================
Coverage ? 68.69%
=========================================
Files ? 7
Lines ? 444
Branches ? 95
=========================================
Hits ? 305
Misses ? 85
Partials ? 54
Continue to review full report at Codecov.
|
Instead use UserWarning, and in pytest.ini set a filter on the warning message.
johnomotani
added
the
testing
Test geometries
xbout/tests/test_against_collect.py
Outdated
|
|
||
| from boutdata import collect |
There was a problem hiding this comment.
Does this mean anyone who wants to run the test suite has to have boutdata.collect on their machine? If so I think that's okay.
There was a problem hiding this comment.
I guess it does. It would probably be good to make it optional - the travis tests already pip-install boutdata, so will be checking this test even if users don't.
|
I've just realised there may be an issue with where the grid data is stored, because for the plotting we want to attach it to BoutDataArrays also. There must be a way around this - I'm thinking about it at the moment, but this isn't quite ready to merge yet. |
Previously, a gridfile was loaded and stored as an attribute of the Dataset and DataArrays, then it was attempted to store the grid Dataset as a member of the BoutDataset accessor (this does not work because accessors are not permanent). As of BOUT++ v4.3.0, the only information we need to load from the grid file is some coordinates (e.g. psixy, Rxy, Zxy for 'toroidal' geometry), so we can just open the grid file, set the coordinates and not save the grid. This has the advantage that DataArrays inherit the coordinates from their parent Dataset without having to do special things when loading, or causing problems when saving to netCDF.
We no longer merge grid data from a grid file into the Dataset, so this test is not needed.
MXG is not contained in the grid file, so needs to be passed as an argument to _open_grid: read from the Dataset if one is being opened, otherwise defaults to 2.
If boutdata.collect, 'old collect', is not available, then skip tests comparing to it. Ensures users who do not install old python tools can still run the tests.
This PR uses the changes in #36, so should be reviewed after that is merged.
It uses
test_load.create_bout_ds_listto finish implementing thetest_against_collecttests, and removes the remaining dependencies on the binary files inxbout/tests/data/dump_files.It also adds a
pytest.inifile to ignore some expectedxBOUTwarnings and some warnings fromxarraythat (I hope) don't affect us. Suppressing warnings reduces the console output ofpytestto less than one screen, making it much easier to read.