Test geometries #58
Test geometries #58
Conversation
Implement 1D animations, and refactor generation of test data
Previously the return statement was missing, leading to a bug.
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.
|
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-10-28 20:40:07 UTC |
xbout/load.py
Outdated
|
|
||
| if gridfilepath is not None: | ||
| print('here in load', ds.attrs) | ||
| ds.attrs["_grid"] = open_boutdataset(gridfilepath, chunks=chunks, |
There was a problem hiding this comment.
If we store the grid data in the attrs like this then the dataset won't be serializable to netcdf. If the grid variables are just normal variables then it would be fine.
There was a problem hiding this comment.
The _grid is possibly only needed temporarily - the current use is that some variables are read from it and put into the dataset in add_toroidal_geometry_coords() and add_s_alpha_geometry_coords(). I'm inclined to leave it available once it's added in case we want to get other info out of it later, but for the way it's being used here we could happily just drop/ignore the _grid dataset when saving to netcdf, as any needed variables have been copied to the main dataset.
There was a problem hiding this comment.
...so the way I've implemented so far isn't a good idea, as @TomNicholas says. Suggestions for a replacement welcome!
There was a problem hiding this comment.
I've fixed this problem by making the _grid dataset a member of the BoutDataset accessor, rather than being in attrs, and added saving to netCDF to the tests of 'toroidal' and 's-alpha' geometries.
I found a problem, which might well be an issue to address later: a variable that's added as a 'coordinate' shouldn't have metadata, or when saving to netCDF there will be an error because of not being able to save a dict to netCDF. Adding the variable as a coordinate is the only thing I currently need to do with the variables read from the grid file, so I've used _open_grid instead of open_boutdataset to open the grid without adding metadata. That's OK, but what if:
- we want to use a variable from the grid file as something other than a coordinate
- could be useful to have a
bout.addVariablemethod, that adds metadata to the variable as well as adding it?
- could be useful to have a
- we want to use a variable from, or calculated from, the dataset as a coordinates
- do we need an
addCoordinates()method that would strip metadata and then callset_coords()?
- do we need an
Codecov Report
@@ Coverage Diff @@
## fix-test-against-collect #58 +/- ##
===========================================================
Coverage ? 69.14%
===========================================================
Files ? 8
Lines ? 444
Branches ? 93
===========================================================
Hits ? 307
Misses ? 85
Partials ? 52
Continue to review full report at Codecov.
|
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.
johnomotani
added
the
testing
Builds on #55, so merging into
fix-test-against-collect.Adds unit tests for loading datasets with 'toroidal' or 's-alpha' geometries. These show the need to be able to pass a grid file to get
psixy,Rxy,Zxy,hthevariables, which are not needed by BOUT++ so are never loaded or saved to the dump files. Therefore reinstates the option to pass agridfilepath, but now the grid file can be opened with justopen_boutdataset🎉Also includes a work-around to tell Travis to exclude xarray-0.14.0, which fixes the Travis tests.
Resolves #17.