-
Notifications
You must be signed in to change notification settings - Fork 10
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
Define Regions and add method to get BoutDataArrays in a region #107
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Including a method of BoutDataArray to get data for a single region.
Guard cells taken from a neighbouring region may have coordinates that are not continuous with the coordinates of the region they are being added to. Work around this by replacing the coordinate values of the guard cells being added with the coordinate values of the adjacent couple of points in the global grid (which have values continuous with those in the region being added to). Adds some utility methods to Region to get slices to select the values to fill the guard cells with.
Allows BoutDataArray methods to use the regions.
Required keyword arguments are clearer than having many positional arguments. Rename the xinner/xouter/ylower/yupper arguments with a suffix _ind as they are indices and will add coordinate values as well.
Now that there are 'regions' elsewhere in the code, it is better to change the variable name to make clear that the members of da_regions are the parts of the DataArray in each region (not the regions themselves).
Also re-order arguments to Region constructor.
Also store regions in OrderedDict instead of dict.
This is the appropriate place, as regions do not necessarily exist for all geometries.
Combinations like jyseps1_2==ny_inner-1 and similar are not supported.
jyseps1_2 should be <=jyseps2_2.
OrderedDicts cannot be saved to netCDF. Regions can be reconstructed from metadata anyway.
Just use the index values as the coordinate values. Having a global coordinate is useful for remembering positions and combining regions.
s-alpha adds an 'r' coordinate for the x-dimension. Remove the global 'x' coordinate added by add_toroidal_geometry_coords in this case so that we can rename the 'x' dimension to 'r'. 'r' provides a global coordinate, so we do not need the global 'x' coordinate.
xarray.DataArray.transpose() gives a FutureWarning about transposing coordinates. Can silence this by passing 'transpose_coords=True', which means we use the future default behaviour.
'Different topologies' means setting up ixseps*, jyseps*, ny_inner. Useful for testing regions, etc.
'core' and 'limiter' topologies may not have neighbouring points to get guard cell coordinates from if the Dataset was opened with keep_yboundaries=False. Raise an informative exception if this occurs.
Need to not count extra points in the global grid for the upper target boundary cells if there is no upper target.
Should not have been changed to exclude boundary points.
Avoids name conflict with 'region' local variable.
...to follow PEP8 naming convention.
Equivalent to self.* except it has to create a new accessor instance.
...to follow PEP8 naming convention.
...to follow PEP8 naming convention.
The dict can be passed directly to isel().
There was previously a lot of logic to do with getting guard cells around a region in BoutDataArray.from_region(). Moving this into separate utility functions in region.py makes from_region() clearer and collects the region logic into the same file.
ZedThree
reviewed
Mar 24, 2020
Makes BoutDataArray.from_region cleaner.
Add _x and _y in the names of connection_* members of Region, to be clearer about which direction they belong to. Also rename the constructor arguments from connect_* to connection_*.
This was referenced Mar 28, 2020
Open
If a Dataset with a geometry applied was saved, it already has coordinates created, and only needs regions re-creating, so handle specially in add_toroidal_geometry_coords() and add_s_alpha_geometry_coords().
Needed for tests on Datasets with Region attributes.
@TomNicholas I think this PR is ready for review/merge now. |
Codecov Report
@@ Coverage Diff @@
## master #107 +/- ##
===========================================
+ Coverage 53.16% 70.22% +17.06%
===========================================
Files 11 12 +1
Lines 1217 1488 +271
Branches 246 299 +53
===========================================
+ Hits 647 1045 +398
+ Misses 496 353 -143
- Partials 74 90 +16
Continue to review full report at Codecov.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It will be useful to operate on or plot variables in a region, within which the grid is contiguous. This PR provides a
Region
class to define regions (indices of their boundaries, connections to neighbouring regions). It also adds a methodBoutDataArray.fromRegion()
to get a variable in a region, including guard cells correctly filled from the connections of the region (not the adjacent points in the global array).Includes #106, because it uses the
join
argument toxr.concat
.This is a fairly big PR, but only 835 of the lines added are 'code', the rest are for the tests.