iris icon indicating copy to clipboard operation
iris copied to clipboard

2D coord bounds handling

Open stephenworsley opened this issue 5 years ago • 9 comments

~~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).~~

stephenworsley avatar Sep 16 '19 11:09 stephenworsley

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.

stephenworsley avatar Sep 16 '19 16:09 stephenworsley

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.

stephenworsley avatar Sep 17 '19 09:09 stephenworsley

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)

stephenworsley avatar Sep 18 '19 10:09 stephenworsley

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.

stephenworsley avatar Sep 18 '19 11:09 stephenworsley

I have suggested a fix to everything I currently think is an issue in #3404.

stephenworsley avatar Sep 18 '19 11:09 stephenworsley

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..

pp-mo avatar Oct 04 '19 09:10 pp-mo

See #3345

cpelley avatar Jan 22 '20 14:01 cpelley

@stephenworsley has confirmed that there is still a problem here, but would like to describe it better before discussing further.

trexfeathers avatar Jul 27 '22 09:07 trexfeathers

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.

stephenworsley avatar Jul 28 '22 08:07 stephenworsley

closed by #4975

stephenworsley avatar Jun 29 '23 15:06 stephenworsley