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.
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
Cice grid #5
Cice grid #5
Changes from 22 commits
a043e00
fe4c538
4a0477e
36b70e1
5b1bd80
34fb292
c39dc46
7ecf9eb
e4da7c2
b6f1d01
c2fd7c9
b2cf750
8cc463a
224697a
cc4ebff
dd29aa5
9cb25e3
97a4a87
47edb5d
94903cf
8be5509
6b8c644
29c2291
d836e70
76665f7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check warning on line 73 in om3utils/cice_grid.py
Codecov / codecov/patch
om3utils/cice_grid.py#L73
Check warning on line 77 in om3utils/cice_grid.py
Codecov / codecov/patch
om3utils/cice_grid.py#L75-L77
Check warning on line 80 in om3utils/cice_grid.py
Codecov / codecov/patch
om3utils/cice_grid.py#L80
Check warning on line 82 in om3utils/cice_grid.py
Codecov / codecov/patch
om3utils/cice_grid.py#L82
Check warning on line 84 in om3utils/cice_grid.py
Codecov / codecov/patch
om3utils/cice_grid.py#L84
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.
Am I right in guessing that isel slices start from 2 rather than 0 because 0 corresponds to the NE corner of cells that are not part of the actual grid?
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.
If so, perhaps add a comment to that effect as it is a bit confusing otherwise
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.
Yes that's right, in the old OM2 grid
ulat
starts from zero led to mismatch in longitudesThere 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 did a bit of a refactor to hopefully clarify. Its hard to write this stuff concisely in comments.
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.
Do we just want a reversed copy of the wrapped top row here, or have I misunderstood?
(Or perhaps the folded top row should be concatenated before wrapping? The order of these operations affects the index of the tripole singularities - see fig 9.9 of Griffies - note that the singularities fall on u points 0 and ni/2)
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 think I tried this suggestion and the resulting areas were offset by 1 cell ... I will have to investigate. I am confident the areas from this method are right, but maybe the method is wrong.
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.
Ah ...
xarray is confusing sometimes.
ds.area.isel(ny=-1, nx=[-1, 0])
andds.area.isel(ny=-1, nx=[0,-1])
are equal :)Anyway the slightly better version of this line would be:
xr.concat([ds.area.isel(ny=-1, nx=slice(-2, None, -1)), ds.area.isel(ny=-1, nx=-1)], dim="nx")
But the even better version, is to fold first, then wrap, and then its generally clearer in total!
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.
Changed to fold first, then wrap