-
Notifications
You must be signed in to change notification settings - Fork 168
Implementing FieldSet.from_nemo() #2447
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
Conversation
And update of tutorial_nemo_curvilinear
And updating tutorial
ANd expading comments on loading in the dataset
So that users have access to UV even if there's also a W field
following on 2d9640e
To use same ordering as in XLinear
This requires smaller selection_dict for the isel, so hopefully faster code
VeckoTheGecko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before I further review this PR, I think it would be good to have a clear picture of what the intended API is. Perhaps this is easier to discuss in person
| vertical_dimensions=(sgrid.DimDimPadding("z_center", "depth", sgrid.Padding.HIGH),), | ||
| ).to_attrs(), | ||
| ) | ||
| fieldset = FieldSet.from_sgrid_conventions(ds, mesh="spherical") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at ds.info() at this point of execution I get the following (using parcels.download_example_dataset("NemoCurvilinear_data"))
ds.info()
xarray.Dataset {
dimensions:
depth = 1 ;
time = 1 ;
y = 1021 ;
x = 1442 ;
variables:
int64 depth(depth) ;
depth:axis = Z ;
float32 V(depth, time, y, x) ;
V:actual_range = [-0.9999932 0.9999932] ;
float64 time(time) ;
time:units = unknown ;
time:valid_min = 3.0 ;
time:valid_max = 3.0 ;
time:axis = T ;
float32 U(depth, time, y, x) ;
U:actual_range = [-1. 1.] ;
float32 fmask(depth, y, x) ;
float32 tmask(depth, y, x) ;
float32 umask(depth, y, x) ;
float32 vmask(depth, y, x) ;
float64 glamf(y, x) ;
glamf:axis = X ;
float64 glamt(depth, y, x) ;
float64 glamu(depth, y, x) ;
float64 glamv(depth, y, x) ;
float64 gphif(y, x) ;
gphif:axis = Y ;
float64 gphit(depth, y, x) ;
float64 gphiu(depth, y, x) ;
float64 gphiv(depth, y, x) ;
int64 x(x) ;
int64 y(y) ;
int64 grid() ;
grid:cf_role = grid_topology ;
grid:topology_dimension = 2 ;
grid:node_dimensions = glamf gphif ;
grid:face_dimensions = x_center:x (padding:low) y_center:y (padding:low) ;
grid:vertical_dimensions = z_center:depth (padding:high) ;
[//](https://github.com/Parcels-code/Parcels/pull/2447/changes) global attributes:
:About = Created by SOSIE interpolation environement => https://github.com/brodeau/sosie/ ;
}
U and V are both defined on (depth, time, y, x) here rather than a staggered grid - so the generated dataset doesn't match the metadata attached to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that this
ds = xr.merge([ds_fields, ds_coords[["glamf", "gphif"]]])
fieldset = parcels.FieldSet.from_nemo(ds)is the way to go.
- Merging before passing to
from_nemoresults in variables (e.g., U and V) which are both defined on dimensions named x and y to be considered to be defined on the same points rather than on a staggered grid - Having
from_nemoreturn a fieldset instead of the dataset prevents the user from making additional changes to the dataset (e.g., fixing their metadata)
I think a better API would be to have from_nemo take in datasets, and then merge them together under the hood. This way we know that the origin datasets are different and can treat them accordingly (i.e., stagger them). If this then returns a dataset then the user can edit and slice further.
And also separating offset calculation into its own helper function
| yi_o = np.clip(yi + offsets["Y"], 0, ydim - 1) | ||
| yi_full = np.tile(np.array([yi_o, yi_o]).flatten(), lenT) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm doubting a bit whether the offset incorporation should be done here in the interpolators, or already in the xgrid.search (where the particle indices are calculated). It depends on whether our internal model always assumes the reference cell node to be the lower left corner, or whether we want to follow the indexing of the respective model.
@VeckoTheGecko, what do you think?
|
todo Nick
|
| ).to_attrs(), | ||
| ) | ||
| fieldset = FieldSet.from_sgrid_conventions(ds, mesh="spherical") | ||
| fieldset.V.units = GeographicPolar() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@erikvansebille I'm not sure why this is GeographicPolar and not Geographic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's because the V velocity is not really the meridional velocity, but the velocity in the j-direction, which can also have a zonal component. The Jacobian transformation works on both velocities at the same time
Without this change, the particles in the curvilinear tests didn't move exactly zonal; so the proof is in the pudding?
This PR implements a
FieldSet.from_nemo()method, based on theFieldSet.from_sgrid_conventions()method.It also improves the
CGrid_Velocityinterpolation method, using thexgcm.gridinformation by taking into account the "left" versus "right" indexing of each axis.Todo
tutorial_nemo_3D.ipynbnotebook fromexamples-v3toexamples(but leaving there until after reviewing, as git diff won't work after the move)