Skip to content

Commit f6055fa

Browse files
authored
Fix geodesic coord resolution to prefer real lat/lon over numeric y/x (#2903)
* Fix geodesic coord resolution to prefer real lat/lon over numeric y/x (#2896) _find_coord accepted the numeric dimension coord before scanning for a coord named latitude/longitude, so a curvilinear raster with numeric y/x index coords plus real 2-D lat/lon coords used the pixel indices as lat/lon. The range check did not catch it because small indices fall inside the geographic ranges. Prefer an explicitly named lat/lon coord over the dimension coord. This covers slope, aspect, and surface_distance, which share _extract_latlon_coords. Add curvilinear coverage (dims y/x + 2-D lat/lon + numeric y/x) across the four backends. * Address review nit: document explicit-name coord resolution order (#2896)
1 parent 63da895 commit f6055fa

2 files changed

Lines changed: 143 additions & 6 deletions

File tree

xrspatial/tests/test_geodesic_slope.py

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,40 @@ def _make_geo_raster(elev, lat_start, lat_end, lon_start, lon_end,
4343
return raster
4444

4545

46+
def _make_curvilinear_raster(elev, lat_start, lat_end, lon_start, lon_end,
47+
backend='numpy', chunks=(3, 3)):
48+
"""Build a curvilinear DataArray: dims ('y', 'x') with numeric y/x index
49+
coords AND real 2-D lat/lon coords over the same geographic grid.
50+
51+
This is the layout that exposed the coordinate-resolution bug: the numeric
52+
y/x pixel-index coords must not be used as lat/lon when real lat/lon coords
53+
are present.
54+
"""
55+
H, W = elev.shape
56+
lat1d = np.linspace(lat_start, lat_end, H)
57+
lon1d = np.linspace(lon_start, lon_end, W)
58+
lon2d, lat2d = np.meshgrid(lon1d, lat1d)
59+
raster = xr.DataArray(
60+
elev.astype(np.float64),
61+
dims=['y', 'x'],
62+
coords={
63+
'y': np.arange(H, dtype=np.float64),
64+
'x': np.arange(W, dtype=np.float64),
65+
'lat': (('y', 'x'), lat2d),
66+
'lon': (('y', 'x'), lon2d),
67+
},
68+
)
69+
70+
if 'cupy' in backend:
71+
import cupy
72+
raster.data = cupy.asarray(raster.data)
73+
74+
if 'dask' in backend and da is not None:
75+
raster.data = da.from_array(raster.data, chunks=chunks)
76+
77+
return raster
78+
79+
4680
def _flat_surface(H=6, W=8, elev=500.0):
4781
"""Constant-elevation surface — slope should be 0 everywhere interior."""
4882
return np.full((H, W), elev, dtype=np.float64)
@@ -108,6 +142,87 @@ def test_north_tilted_has_positive_slope(self):
108142
assert np.all(interior > 0)
109143

110144

145+
class TestGeodesicSlopeCurvilinear:
146+
"""Curvilinear layout: dims ('y', 'x') with numeric y/x index coords plus
147+
real 2-D lat/lon coords. The geodesic path must use the lat/lon coords, not
148+
the pixel indices, so the result must match the equivalent 1-D lat/lon grid.
149+
150+
Pixel indices (0..N) fall inside the accepted geographic ranges, so the
151+
range validation does not catch the mistake — only the slope value does.
152+
"""
153+
154+
def test_curvilinear_matches_1d_latlon(self):
155+
elev = _east_tilted_surface(H=8, W=10, grade=100.0)
156+
r_curv = _make_curvilinear_raster(elev, 40.0, 41.0, 10.0, 11.0)
157+
r_ref = _make_geo_raster(elev, 40.0, 41.0, 10.0, 11.0)
158+
s_curv = slope(r_curv, method='geodesic')
159+
s_ref = slope(r_ref, method='geodesic')
160+
np.testing.assert_allclose(
161+
s_curv.values, s_ref.values, rtol=1e-5, equal_nan=True
162+
)
163+
164+
def test_curvilinear_ignores_pixel_index_coords(self):
165+
"""Slope must reflect the real geographic grid, not the 0..N indices.
166+
167+
Using the pixel indices as lat/lon collapses the east tilt to a tiny
168+
value (~0.007 vs ~0.067), so a correct interior slope is the signal.
169+
"""
170+
elev = _east_tilted_surface(H=8, W=10, grade=100.0)
171+
r_curv = _make_curvilinear_raster(elev, 40.0, 41.0, 10.0, 11.0)
172+
interior = slope(r_curv, method='geodesic').values[1:-1, 1:-1]
173+
assert np.all(np.isfinite(interior))
174+
assert np.all(interior > 0.05)
175+
176+
177+
@dask_array_available
178+
class TestGeodesicSlopeCurvilinearDask:
179+
180+
def test_curvilinear_numpy_equals_dask(self):
181+
elev = _east_tilted_surface(H=8, W=10, grade=100.0)
182+
r_np = _make_curvilinear_raster(elev, 40.0, 41.0, 10.0, 11.0,
183+
backend='numpy')
184+
r_da = _make_curvilinear_raster(elev, 40.0, 41.0, 10.0, 11.0,
185+
backend='dask+numpy', chunks=(4, 5))
186+
s_np = slope(r_np, method='geodesic')
187+
s_da = slope(r_da, method='geodesic')
188+
np.testing.assert_allclose(
189+
s_np.values, s_da.values, rtol=1e-5, equal_nan=True
190+
)
191+
192+
193+
@cuda_and_cupy_available
194+
class TestGeodesicSlopeCurvilinearCupy:
195+
196+
def test_curvilinear_numpy_equals_cupy(self):
197+
elev = _east_tilted_surface(H=8, W=10, grade=100.0)
198+
r_np = _make_curvilinear_raster(elev, 40.0, 41.0, 10.0, 11.0,
199+
backend='numpy')
200+
r_cu = _make_curvilinear_raster(elev, 40.0, 41.0, 10.0, 11.0,
201+
backend='cupy')
202+
s_np = slope(r_np, method='geodesic')
203+
s_cu = slope(r_cu, method='geodesic')
204+
np.testing.assert_allclose(
205+
s_np.values, s_cu.data.get(), rtol=1e-5, equal_nan=True
206+
)
207+
208+
209+
@dask_array_available
210+
@cuda_and_cupy_available
211+
class TestGeodesicSlopeCurvilinearDaskCupy:
212+
213+
def test_curvilinear_numpy_equals_dask_cupy(self):
214+
elev = _east_tilted_surface(H=8, W=10, grade=100.0)
215+
r_np = _make_curvilinear_raster(elev, 40.0, 41.0, 10.0, 11.0,
216+
backend='numpy')
217+
r_dc = _make_curvilinear_raster(elev, 40.0, 41.0, 10.0, 11.0,
218+
backend='dask+cupy', chunks=(4, 5))
219+
s_np = slope(r_np, method='geodesic')
220+
s_dc = slope(r_dc, method='geodesic')
221+
np.testing.assert_allclose(
222+
s_np.values, s_dc.data.compute().get(), rtol=1e-5, equal_nan=True
223+
)
224+
225+
111226
class TestGeodesicSlopeLatitudeInvariance:
112227
"""Same physical slope at equator vs 60N should give similar geodesic slope."""
113228

xrspatial/utils.py

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -935,6 +935,11 @@ def warn_if_unit_mismatch(agg: xr.DataArray) -> None:
935935
# Known dimension / coordinate names (lower-cased for matching)
936936
_LAT_NAMES = {'lat', 'latitude', 'y'}
937937
_LON_NAMES = {'lon', 'longitude', 'x'}
938+
# Names that unambiguously mean geographic lat/lon. These take precedence
939+
# over a numeric dimension coord so a curvilinear raster with numeric y/x
940+
# index coords plus real lat/lon coords resolves to the lat/lon coords.
941+
_EXPLICIT_LAT_NAMES = {'lat', 'latitude'}
942+
_EXPLICIT_LON_NAMES = {'lon', 'longitude'}
938943

939944

940945
def _extract_latlon_coords(agg: xr.DataArray):
@@ -965,9 +970,11 @@ def _extract_latlon_coords(agg: xr.DataArray):
965970
dim_y, dim_x = agg.dims[-2], agg.dims[-1]
966971

967972
# --- locate lat coordinate ---
968-
lat_coord = _find_coord(agg, dim_y, _LAT_NAMES, 'latitude')
973+
lat_coord = _find_coord(agg, dim_y, _LAT_NAMES, _EXPLICIT_LAT_NAMES,
974+
'latitude')
969975
# --- locate lon coordinate ---
970-
lon_coord = _find_coord(agg, dim_x, _LON_NAMES, 'longitude')
976+
lon_coord = _find_coord(agg, dim_x, _LON_NAMES, _EXPLICIT_LON_NAMES,
977+
'longitude')
971978

972979
lat_vals = np.asarray(lat_coord.values, dtype=np.float64)
973980
lon_vals = np.asarray(lon_coord.values, dtype=np.float64)
@@ -994,15 +1001,30 @@ def _extract_latlon_coords(agg: xr.DataArray):
9941001
return lat_2d, lon_2d
9951002

9961003

997-
def _find_coord(agg, dim_name, known_names, label):
998-
"""Find a coordinate matching *dim_name* or one of *known_names*."""
999-
# 1) Try the dimension name directly
1004+
def _find_coord(agg, dim_name, known_names, explicit_names, label):
1005+
"""Find a coordinate matching *dim_name* or one of *known_names*.
1006+
1007+
A coordinate whose name is unambiguously geographic (*explicit_names*,
1008+
e.g. ``lat``/``longitude``) is preferred over the dimension coord. This
1009+
keeps a curvilinear raster with numeric ``y``/``x`` index coords plus
1010+
real lat/lon coords from silently using the pixel indices as lat/lon.
1011+
If several explicit names are present (e.g. both ``lat`` and
1012+
``latitude``), the first one in coord order wins.
1013+
"""
1014+
# 1) Prefer an explicitly named geographic coordinate (lat/lon).
1015+
for name in agg.coords:
1016+
if str(name).lower() in explicit_names:
1017+
coord = agg.coords[name]
1018+
if np.issubdtype(coord.dtype, np.number):
1019+
return coord
1020+
1021+
# 2) Fall back to the dimension name directly.
10001022
if dim_name in agg.coords:
10011023
coord = agg.coords[dim_name]
10021024
if np.issubdtype(coord.dtype, np.number):
10031025
return coord
10041026

1005-
# 2) Scan all coords for a known name
1027+
# 3) Scan all coords for any other known name (e.g. y/x).
10061028
for name in agg.coords:
10071029
if str(name).lower() in known_names:
10081030
coord = agg.coords[name]

0 commit comments

Comments
 (0)