iris
iris copied to clipboard
2D coord bounds handling
~~Currently, a 2D coordinate point is expected to have 4 bounds. There ought to be 8. Assuming the 2D variable is defined over lattitude and longitude, there should be 4 latbounds and 4 lonbounds. http://cfconventions.org/Data/cf-conventions/cf-conventions-1.7/cf-conventions.html#cell-boundaries~~
The current way these bounds are handled is incomplete. _discontiguity_in_bounds should be making more comparisons. It is currently comparing bounds[*,*,0]
with bounds[*,*,1]
and bounds[*,*,0]
with bounds[*,*,3]
. it should also be comparing bounds[*,*,1]
with bounds[*,*,2]
and bounds[*,*,2]
with bounds[*,*,3]
. These comparisons should be done for both latbounds and lonbounds.
~~It's also worth noting that it's possible to end up with a coordinate which has bounds in one dimension but not another. This is in fact the case for the gallery example https://scitools.org.uk/iris/docs/v1.9.2/examples/General/cross_section.html . It's unclear how such cases should be treated (they are currently unable to be handled by pcolor/pcolormesh. That is perhaps the correct approach, but in order for something like that to be possible, it seems like there would need to be some more thought put into the structure of bounds handling).~~
My intuition is to have the shape of 2D bounds be (*,*,2,4)
. I've taken a quick look at where in the code may have to change and I came up with the following list of functions:
coords._get_2d_coord_bound_grid
coords.Cell.contains_point
coords.Coord._bounds_setter
coords.Coord._sanity_check_bounds
coords.Coord._discontiguity_in_bounds
coords.Coord.contiguous_bounds
coords.Coord.nbounds
coords.Coord._guess_bounds
coords.DimCoord._new_bounds_requirements
analysis._grid_angles
And potentially a bunch of places with load/save rules.
My intuition is to have the shape of 2D bounds be
(*,*,2,4)
.
Having re-read the CF Conventions, I've reconsidered my position on this. It seems like CF Conventions is consistent with what we are doing currently. With that said, it still seems like we are not comparing enough points for discontiguity. I'm still unsure how to interpret CF Conventions for derived coodinates or how 2D latlon grid bounds should behave when sliced.
Another concern I have about _discontiguity_in_bounds()
is that the line
https://github.com/SciTools/iris/blob/d0213d8b567d0b83ac4e3fe52b86d13c6a5e8e5e/lib/iris/coords.py#L1078
seems to be inappropriately comparing a boolean array to a float. I feel like it would make more sense to either replace diffs_along_axis
with diffs_between_cells
or else remove this line and replace the next line with
contiguous_along_axis = not np.any(diffs_along_axis)
Also, there seems to be some inconsistency as to what is returned by _discontiguity_in_bounds()
as diffs
. For 1D coordinates it seems to be returning an array of floats while for 2D bounds it is returning an array of booleans. It seems like wherever _discontiguity_in_bounds()
is called, it is expecting diffs to contain booleans.
I have suggested a fix to everything I currently think is an issue in #3404.
inappropriately comparing a boolean array to a float.
Definitely a bug. It looks very much like a change to the implementation but then failing to delete a line of the previous version !
Thanks for making the effort here..
See #3345
@stephenworsley has confirmed that there is still a problem here, but would like to describe it better before discussing further.
I think this issue has got garbled somewhat by my initial observation being mistaken, but my additional observations still (I think) being valid. There are two problems I've identified with 2D contiguous bounds checking I've found:
- Not enough comparisons are being made. Currently, the only comparisons done are between the vertices indexed at 0 and 1 and the vertices indexed at 0 and 3. i.e. only the following comparisons are made:
3---2 3---2
| | | |
0---1 + 0---1
+
3---2
| |
0---1
In order to do a full comparison, we would also need to compare the vertices indexed at 1 and 2 and the vertices indexed at 2 and 3:
3---2 + 3---2
| | | |
0---1 + 0---1
+ +
3---2
| |
0---1
- The logic involved in processing the diffs is nonsensical. Specifically this line: https://github.com/SciTools/iris/blob/d0213d8b567d0b83ac4e3fe52b86d13c6a5e8e5e/lib/iris/coords.py#L1078
Both of these are addressed by #3404 which has currently stalled.
closed by #4975