Skip to content
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

Fix bugs following geoutils update #291

Merged
merged 24 commits into from
Sep 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
cd0066f
Fix issues with _get_rio_attrs
adehecq Aug 25, 2022
bc5ba01
Replace use of self.ds and self._update
adehecq Aug 25, 2022
ac14e63
Revert to using latest geoutils
adehecq Aug 25, 2022
f47403d
Fix issue with richdem functions
adehecq Aug 25, 2022
ec6dba5
Fix a test_coreg test that does not seem to be related to geoutils
adehecq Aug 25, 2022
bf5a108
Fix typing and update naming for raster to richdem arrays
rhugonnet Aug 25, 2022
cd58610
Add test for example data
rhugonnet Aug 25, 2022
4221dd0
Update the values of nodata to before the MemoryFile PR
rhugonnet Aug 25, 2022
aaa577a
Fix coreg processing (relique of old GeoUtils nodata)
rhugonnet Aug 25, 2022
9894fee
Pass directly Raster to get_array_and_mask
rhugonnet Aug 25, 2022
c888209
Import future annotations for using tuple in typing
rhugonnet Aug 25, 2022
b67b381
Try to fix issue in examples with number of nodata values
adehecq Aug 26, 2022
eefc10f
Try with the absolute tolerance of the input DEMs
rhugonnet Aug 26, 2022
f7bc249
Test with array_equal by passing float32 values with full precision
rhugonnet Aug 26, 2022
17d759d
Try to add a random state to scipy.least_sq to get the exact same ddem
rhugonnet Aug 26, 2022
c1182c6
Shorten and clarify output formatting
rhugonnet Aug 26, 2022
6a56039
Remove the change of nodata in -9999
rhugonnet Aug 26, 2022
871b3ac
Update values in test_examples.py for longyerbyen dataset
adehecq Sep 1, 2022
6be44a3
Revert truevals in test_examples and use approx instead.
adehecq Sep 1, 2022
b30f224
Use approx instead of exact equality in test_coreg.py
adehecq Sep 1, 2022
bd784c1
Modify geoutils name for fetching from git
rhugonnet Sep 1, 2022
cf65abd
Ensure processed data is overwritten during GitHub Actions
rhugonnet Sep 3, 2022
ad4e438
Add existence condition for removing processed files
rhugonnet Sep 3, 2022
2e935ea
Force older rasterio version to see if it triggers the nodata issue
rhugonnet Sep 3, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/python-package.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ jobs:
# pytest will run asynchronously, so test data need to be downloaded first.
- name: Download example data
run: |
python -c "from xdem.examples import *; download_longyearbyen_examples(overwrite=False)"
python -c "from xdem.examples import *; download_longyearbyen_examples(overwrite=True)"
- name: Test with pytest
run: |
pip install pytest-cov coveralls
Expand Down
5 changes: 2 additions & 3 deletions dev-environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ dependencies:
- matplotlib
- opencv
- pyproj
- rasterio
- rasterio<=1.2.10
- scipy
- tqdm
- scikit-image
Expand All @@ -33,6 +33,5 @@ dependencies:
- sphinx-gallery

- pip:
- geoutils==0.0.8
# - git+https://github.com/GlacioHack/GeoUtils.git
- git+https://github.com/GlacioHack/geoutils.git

2 changes: 1 addition & 1 deletion dev-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ scipy
rasterio
pyproj
tqdm
git+https://github.com/GlacioHack/GeoUtils.git
git+https://github.com/GlacioHack/geoutils.git
scikit-gstat
flake8
opencv-contrib-python
Expand Down
2 changes: 1 addition & 1 deletion environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,4 @@ dependencies:
- proj-data
- scikit-gstat>=0.6.8
- pytransform3d
- geoutils>=0.0.5
- geoutils>=0.0.8
17 changes: 13 additions & 4 deletions tests/test_coreg.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,9 +199,16 @@ def test_nuth_kaab(self):
transform=self.ref.transform, verbose=self.fit_params["verbose"])

# Make sure that the estimated offsets are similar to what was synthesized.
assert abs(nuth_kaab._meta["offset_east_px"] - pixel_shift) < 0.03
assert abs(nuth_kaab._meta["offset_north_px"]) < 0.03
assert abs(nuth_kaab._meta["bias"] + bias) < 0.03
assert nuth_kaab._meta["offset_east_px"] == pytest.approx(pixel_shift, abs=0.03)
assert nuth_kaab._meta["offset_north_px"] == pytest.approx(0, abs=0.03)
assert nuth_kaab._meta["bias"] == pytest.approx(-bias, 0.03)

# Check that the random states forces always the same results
# Note: in practice, the values are not exactly equal for different OS/conda config
assert nuth_kaab._meta["offset_east_px"] == pytest.approx(2.000192638759735, abs=1e-7)
assert nuth_kaab._meta["offset_north_px"] == pytest.approx(-0.0001202906750811198, abs=1e-7)
assert nuth_kaab._meta["bias"] == -5.0


# Apply the estimated shift to "revert the DEM" to its original state.
unshifted_dem = nuth_kaab.apply(shifted_dem, transform=self.ref.transform)
Expand Down Expand Up @@ -566,9 +573,11 @@ def test_coreg_raster_and_ndarray_args(_) -> None:
dem2_a = biascorr_a.apply(dem2.data, dem2.transform)

# Validate that the return formats were the expected ones, and that they are equal.
# Issue - dem2_a does not have the same shape, the first dimension is being squeezed
# TODO - Fix coreg.apply?
assert isinstance(dem2_r, xdem.DEM)
assert isinstance(dem2_a, np.ma.masked_array)
assert np.array_equal(dem2_r, dem2_r)
assert gu.misc.array_equal(dem2_r.data.squeeze(), dem2_a, equal_nan=True)

# If apply on a masked_array was given without a transform, it should fail.
with pytest.raises(ValueError, match="'transform' must be given"):
Expand Down
5 changes: 3 additions & 2 deletions tests/test_dem.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import warnings

import geoutils.georaster as gr
from geoutils.georaster.raster import _default_rio_attrs
import geoutils.satimg as si
import numpy as np
import pyproj
Expand Down Expand Up @@ -44,7 +45,7 @@ def test_init(self):
list_dem = [dem, dem2, dem3, dem4]

# Check all attributes
attrs = [at for at in r._get_rio_attrs() if at not in ['name', 'dataset_mask', 'driver']]
attrs = [at for at in _default_rio_attrs if at not in ['name', 'dataset_mask', 'driver']]
all_attrs = attrs + si.satimg_attrs + xdem.dem.dem_attrs
for attr in all_attrs:
attrs_per_dem = [idem.__getattribute__(attr) for idem in list_dem]
Expand Down Expand Up @@ -83,7 +84,7 @@ def test_copy(self):
# dem_attrs = ['vref', 'vref_grid', 'ccrs']

# using list directly available in Class
attrs = [at for at in r._get_rio_attrs() if at not in ['name', 'dataset_mask', 'driver']]
attrs = [at for at in _default_rio_attrs if at not in ['name', 'dataset_mask', 'driver']]
all_attrs = attrs + si.satimg_attrs + xdem.dem.dem_attrs
for attr in all_attrs:
assert r.__getattribute__(attr) == r2.__getattribute__(attr)
Expand Down
54 changes: 54 additions & 0 deletions tests/test_examples.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
"""Functions to test the example data."""
from __future__ import annotations

import geoutils as gu
import geoutils.spatial_tools
from geoutils import Raster, Vector
import numpy as np
import pandas as pd
import pytest

import xdem
from xdem import examples

def load_examples() -> tuple[Raster, Raster, Vector, Raster]:
"""Load example files to try coregistration methods with."""

ref_dem = Raster(examples.get_path("longyearbyen_ref_dem"))
tba_dem = Raster(examples.get_path("longyearbyen_tba_dem"))
glacier_mask = Vector(examples.get_path("longyearbyen_glacier_outlines"))
ddem = Raster(examples.get_path('longyearbyen_ddem'))

return ref_dem, tba_dem, glacier_mask, ddem

class TestExamples:

ref_dem, tba_dem, glacier_mask, ddem = load_examples()

@pytest.mark.parametrize('rst_and_truevals',
[(ref_dem, np.array([868.6489, 623.42194, 180.57921, 267.30765, 601.67615], dtype=np.float32)),
(tba_dem, np.array([875.2358, 625.0544, 182.9936, 272.6586, 606.2897], dtype=np.float32)),
(ddem, np.array([-0.02423096, -0.71899414, 0.14256287, 1.1018677, -5.9209595], dtype=np.float32))])
def test_array_content(self, rst_and_truevals: tuple[Raster, np.ndarray]):
"""Let's ensure the data arrays in the examples are always the same by checking randomly some values"""

rst = rst_and_truevals[0]
truevals = rst_and_truevals[1]
np.random.seed(42)
values = np.random.choice(rst.data.data.flatten(), size=5, replace=False)

# TODO: understand why the values differ slightly between different OS or python configurations
assert values == pytest.approx(truevals, abs=1e-3)

@pytest.mark.parametrize('rst_and_truenodata',
[(ref_dem, 0),
(tba_dem, 0),
(ddem, 2316)])
def test_array_nodata(self, rst_and_truenodata: tuple[Raster, int]):
"""Let's also check that the data arrays have always the same number of not finite values"""

rst = rst_and_truenodata[0]
truenodata = rst_and_truenodata[1]
mask = gu.spatial_tools.get_array_and_mask(rst)[1]

assert np.sum(mask) == truenodata
15 changes: 8 additions & 7 deletions xdem/coreg.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ def residuals(parameters: tuple[float, float, float], y_values: np.ndarray, x_va
return err

# Estimate the a, b, and c parameters with least square minimisation
np.random.seed(seed=42)
plsq = scipy.optimize.leastsq(func=residuals, x0=initial_guess, args=(y_medians, slice_bounds), full_output=1)

a_parameter, b_parameter, c_parameter = plsq[0]
Expand Down Expand Up @@ -419,7 +420,8 @@ def fit(self: CoregType, reference_dem: np.ndarray | np.ma.masked_array | Raster
transform: Optional[rio.transform.Affine] = None,
weights: Optional[np.ndarray] = None,
subsample: Union[float, int] = 1.0,
verbose: bool = False) -> CoregType:
verbose: bool = False,
random_state: None | np.random.RandomState | np.random.Generator | int = None) -> CoregType:
"""
Estimate the coregistration transform on the given DEMs.

Expand All @@ -430,6 +432,7 @@ def fit(self: CoregType, reference_dem: np.ndarray | np.ma.masked_array | Raster
:param weights: Optional. Per-pixel weights for the coregistration.
:param subsample: Subsample the input to increase performance. <1 is parsed as a fraction. >1 is a pixel count.
:param verbose: Print progress messages to stdout.
:param random_state: Random state or seed number to use for calculations (to fix random sampling during testing)
"""

if weights is not None:
Expand Down Expand Up @@ -576,20 +579,18 @@ def apply(self, dem: np.ndarray | np.ma.masked_array | RasterType,
raise ValueError("Coreg method is non-rigid but has no implemented _apply_func")

# Calculate final mask
final_mask = dem_mask + np.isnan(applied_dem)
final_mask = np.logical_or(dem_mask, np.isnan(applied_dem))

# If the DEM was a masked_array, copy the mask to the new DEM
if hasattr(dem, "mask"):
applied_dem = np.ma.masked_array(applied_dem, mask=final_mask) # type: ignore
# If the DEM was a Raster with a mask, copy the mask to the new DEM
elif hasattr(dem, "data") and hasattr(dem.data, "mask"):
if isinstance(dem, (np.ma.masked_array)):
applied_dem = np.ma.masked_array(applied_dem, mask=final_mask) # type: ignore
else:
applied_dem[final_mask] = np.nan

# If the input was a Raster, return a Raster as well.
if isinstance(dem, gu.Raster):
return dem.from_array(applied_dem, transform, dem.crs, nodata=dem.nodata)
return dem.copy(new_array=applied_dem)
# return dem.from_array(applied_dem, transform, dem.crs, nodata=dem.nodata)

return applied_dem

Expand Down
3 changes: 1 addition & 2 deletions xdem/dem.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,11 +231,10 @@ def to_vref(self, vref_name: str = 'EGM96', vref_grid: str | None = None):

# Transform the grid
transformer = Transformer.from_crs(ccrs_init, ccrs_dest)
meta = self.ds.meta
zz = self.data
xx, yy = self.coords(offset='center')
zz_trans = transformer.transform(xx,yy,zz[0,:])[2]
zz[0,:] = zz_trans

# Update raster
self._update(metadata=meta,imgdata=zz)
self.data = zz
16 changes: 10 additions & 6 deletions xdem/examples.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ def download_longyearbyen_examples(overwrite: bool = False):
# print("Datasets exist")
return

# If we ask for overwrite, also remove the processed test data
if overwrite:
for fn in list(FILEPATHS_PROCESSED.values()):
if os.path.exists(fn):
os.remove(fn)

# Static commit hash to be bumped every time it needs to be.
commit = "321f84d5a67666f45a196a31a2697e22bfaf3c59"
# The URL from which to download the repository
Expand Down Expand Up @@ -93,13 +99,11 @@ def process_coregistered_examples(overwrite: bool =False):
inlier_mask = ~glacier_mask.create_mask(reference_raster)

nuth_kaab = xdem.coreg.NuthKaab()
nuth_kaab.fit(reference_raster.data, to_be_aligned_raster.data,
inlier_mask=inlier_mask, transform=reference_raster.transform)
aligned_raster = nuth_kaab.apply(to_be_aligned_raster.data, transform=reference_raster.transform)
aligned_raster.data[~np.isfinite(aligned_raster)] = reference_raster.nodata
nuth_kaab.fit(reference_raster, to_be_aligned_raster,
inlier_mask=inlier_mask)
aligned_raster = nuth_kaab.apply(to_be_aligned_raster)

diff = reference_raster - \
gu.Raster.from_array(aligned_raster.data, transform=reference_raster.transform, crs=reference_raster.crs, nodata=reference_raster.nodata)
diff = reference_raster - aligned_raster

# Save it so that future calls won't need to recreate the file
os.makedirs(os.path.dirname(FILEPATHS_PROCESSED['longyearbyen_ddem']), exist_ok=True)
Expand Down
2 changes: 1 addition & 1 deletion xdem/spatialstats.py
Original file line number Diff line number Diff line change
Expand Up @@ -914,7 +914,7 @@ def sample_empirical_variogram(values: Union[np.ndarray, RasterType], gsd: float
# First, check all that the values provided are OK
if isinstance(values, Raster):
gsd = values.res[0]
values, mask = get_array_and_mask(values.data)
values, mask = get_array_and_mask(values)
elif isinstance(values, (np.ndarray, np.ma.masked_array)):
values, mask = get_array_and_mask(values)
else:
Expand Down
36 changes: 16 additions & 20 deletions xdem/terrain.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,29 +18,28 @@
_has_rd = False


def _rio_to_rda(ds: rio.DatasetReader) -> rd.rdarray:
def _raster_to_rda(rst: RasterType) -> rd.rdarray:
"""
Get georeferenced richDEM array from rasterio dataset
:param ds: DEM
Get georeferenced richDEM array from geoutils.Raster
:param rst: DEM as raster
:return: DEM
"""
arr = ds.read(1)
rda = rd.rdarray(arr, no_data=ds.get_nodatavals()[0])
rda.geotransform = ds.get_transform()
rda.projection = ds.get_gcps()
arr = rst.data.filled(rst.nodata).squeeze()
rda = rd.rdarray(arr, no_data=rst.nodata)
rda.geotransform = rst.transform.to_gdal()

return rda


def _get_terrainattr_richdem(ds: rio.DatasetReader, attribute='slope_radians') -> np.ndarray:
def _get_terrainattr_richdem(rst: RasterType, attribute='slope_radians') -> np.ndarray:
"""
Derive terrain attribute for DEM opened with rasterio. One of "slope_degrees", "slope_percentage", "aspect",
"profile_curvature", "planform_curvature", "curvature" and others (see RichDEM documentation).
:param ds: DEM
:param rst: DEM as raster
:param attribute: RichDEM terrain attribute
:return:
"""
rda = _rio_to_rda(ds)
rda = _raster_to_rda(rst)
terrattr = rd.TerrainAttribute(rda, attrib=attribute)

return np.array(terrattr)
Expand Down Expand Up @@ -768,10 +767,7 @@ def get_terrain_attribute(
for attr in attributes_using_richdem:
attributes_requiring_surface_fit.remove(attr)

if isinstance(dem, gu.Raster):
# Prepare rasterio.Dataset to pass to RichDEM
ds = dem.ds
else:
if not isinstance(dem, gu.Raster):
# Here, maybe we could pass the geotransform based on the resolution, and add a "default" projection as
# this is mandated but likely not used by the rdarray format of RichDEM...
# For now, not supported
Expand Down Expand Up @@ -842,7 +838,7 @@ def get_terrain_attribute(
if make_slope:

if use_richdem:
terrain_attributes["slope"] = _get_terrainattr_richdem(ds, attribute="slope_radians")
terrain_attributes["slope"] = _get_terrainattr_richdem(dem, attribute="slope_radians")

else:
if slope_method == "Horn":
Expand All @@ -863,10 +859,10 @@ def get_terrain_attribute(

if use_richdem:
# The aspect of RichDEM is returned in degrees, we convert to radians to match the others
terrain_attributes["aspect"] = np.deg2rad(_get_terrainattr_richdem(ds, attribute="aspect"))
terrain_attributes["aspect"] = np.deg2rad(_get_terrainattr_richdem(dem, attribute="aspect"))
# For flat slopes, RichDEM returns a 90° aspect by default, while GDAL return a 180° aspect
# We stay consistent with GDAL
slope_tmp = _get_terrainattr_richdem(ds, attribute="slope_radians")
slope_tmp = _get_terrainattr_richdem(dem, attribute="slope_radians")
terrain_attributes["aspect"][slope_tmp == 0] = np.pi

else:
Expand Down Expand Up @@ -914,7 +910,7 @@ def get_terrain_attribute(
if make_curvature:

if use_richdem:
terrain_attributes["curvature"] = _get_terrainattr_richdem(ds, attribute="curvature")
terrain_attributes["curvature"] = _get_terrainattr_richdem(dem, attribute="curvature")

else:
# Curvature is the second derivative of the surface fit equation.
Expand All @@ -927,7 +923,7 @@ def get_terrain_attribute(
if make_planform_curvature:

if use_richdem:
terrain_attributes["planform_curvature"] = _get_terrainattr_richdem(ds, attribute="planform_curvature")
terrain_attributes["planform_curvature"] = _get_terrainattr_richdem(dem, attribute="planform_curvature")

else:
# PLANC = 2(DH² + EG² -FGH)/(G²+H²)
Expand All @@ -953,7 +949,7 @@ def get_terrain_attribute(
if make_profile_curvature:

if use_richdem:
terrain_attributes["profile_curvature"] = _get_terrainattr_richdem(ds, attribute="profile_curvature")
terrain_attributes["profile_curvature"] = _get_terrainattr_richdem(dem, attribute="profile_curvature")

else:
# PROFC = -2(DG² + EH² + FGH)/(G²+H²)
Expand Down