From bc3b1cc5b58f77d11e2e30a70f6938ac7e0e71c2 Mon Sep 17 00:00:00 2001 From: Romain Hugonnet Date: Mon, 27 Nov 2023 18:09:07 -0800 Subject: [PATCH 1/6] Fix bug and remove _fit_func --- xdem/coreg/base.py | 22 +--------------------- 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/xdem/coreg/base.py b/xdem/coreg/base.py index 191c0e71..7e39f2cb 100644 --- a/xdem/coreg/base.py +++ b/xdem/coreg/base.py @@ -1516,7 +1516,7 @@ def fit( main_args_fit = { "reference_dem": ref_dem, - "dem_to_be_aligned": tba_dem, + "dem_to_be_aligned": tba_dem_mod, "inlier_mask": inlier_mask, "transform": out_transform, "crs": crs, @@ -1544,26 +1544,6 @@ def fit( return self - def _fit_pts_func( - self: CoregType, - ref_dem: NDArrayf | MArrayf | RasterType | pd.DataFrame, - tba_dem: RasterType, - verbose: bool = False, - **kwargs: Any, - ) -> CoregType: - - tba_dem_mod = tba_dem.copy() - - for i, coreg in enumerate(self.pipeline): - if verbose: - print(f"Running pipeline step: {i + 1} / {len(self.pipeline)}") - - coreg._fit_pts_func(ref_dem=ref_dem, tba_dem=tba_dem_mod, verbose=verbose, **kwargs) - coreg._fit_called = True - - tba_dem_mod = coreg.apply(tba_dem_mod) - return self - def _apply_func( self, dem: NDArrayf, From b2c1324d281dbaaa26d5c109dd1366e69f7c9b63 Mon Sep 17 00:00:00 2001 From: Romain Hugonnet Date: Mon, 27 Nov 2023 18:48:58 -0800 Subject: [PATCH 2/6] Re-add the fit_pts func --- xdem/coreg/base.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/xdem/coreg/base.py b/xdem/coreg/base.py index 7e39f2cb..0ce059c1 100644 --- a/xdem/coreg/base.py +++ b/xdem/coreg/base.py @@ -1544,6 +1544,26 @@ def fit( return self + def _fit_pts_func( + self: CoregType, + ref_dem: NDArrayf | MArrayf | RasterType | pd.DataFrame, + tba_dem: RasterType, + verbose: bool = False, + **kwargs: Any, + ) -> CoregType: + + tba_dem_mod = tba_dem.copy() + + for i, coreg in enumerate(self.pipeline): + if verbose: + print(f"Running pipeline step: {i + 1} / {len(self.pipeline)}") + + coreg._fit_pts_func(ref_dem=ref_dem, tba_dem=tba_dem_mod, verbose=verbose, **kwargs) + coreg._fit_called = True + + tba_dem_mod = coreg.apply(tba_dem_mod) + return self + def _apply_func( self, dem: NDArrayf, From 6bdc73441ee0beb4ee0a7ab4726276d5c0a9bf0a Mon Sep 17 00:00:00 2001 From: Romain Hugonnet Date: Mon, 27 Nov 2023 20:48:53 -0800 Subject: [PATCH 3/6] Add test to check pipeline output consistency --- tests/test_coreg/test_base.py | 37 +++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tests/test_coreg/test_base.py b/tests/test_coreg/test_base.py index 2e0bfe73..a6580107 100644 --- a/tests/test_coreg/test_base.py +++ b/tests/test_coreg/test_base.py @@ -676,6 +676,43 @@ def test_coreg_add(self) -> None: vshift5 = vshift3 + vshift3 assert vshift5.to_matrix()[2, 3] == vshift * 4 + def test_pipeline_consistency(self): + """Check that pipelines properties are respected: reflectivity, fusion of same coreg""" + + # Test 1: Fusion of same coreg + # Many vertical shifts + many_vshifts = coreg.VerticalShift() + coreg.VerticalShift() + coreg.VerticalShift() + many_vshifts.fit(**self.fit_params) + aligned_dem, _ = many_vshifts.apply(self.tba.data, transform=self.ref.transform, crs=self.ref.crs) + + # The last steps should have shifts of EXACTLY zero + assert many_vshifts.pipeline[1]._meta["vshift"] == pytest.approx(0, abs=10e-5) + assert many_vshifts.pipeline[2]._meta["vshift"] == pytest.approx(0, abs=10e-5) + + # Many horizontal + vertical shifts + many_nks = coreg.NuthKaab() + coreg.NuthKaab() + coreg.NuthKaab() + many_nks.fit(**self.fit_params) + aligned_dem, _ = many_nks.apply(self.tba.data, transform=self.ref.transform, crs=self.ref.crs) + + # The last steps should have shifts of NEARLY zero + assert many_nks.pipeline[1]._meta["vshift"] == pytest.approx(0, abs=0.01) + assert many_nks.pipeline[1]._meta["offset_east_px"] == pytest.approx(0, abs=0.01) + assert many_nks.pipeline[1]._meta["offset_north_px"] == pytest.approx(0, abs=0.01) + assert many_nks.pipeline[2]._meta["vshift"] == pytest.approx(0, abs=0.01) + assert many_nks.pipeline[2]._meta["offset_east_px"] == pytest.approx(0, abs=0.01) + assert many_nks.pipeline[2]._meta["offset_north_px"] == pytest.approx(0, abs=0.01) + + # Test 2: Reflectivity + # Those two pipelines should give almost the same result + nk_vshift = coreg.NuthKaab() + coreg.VerticalShift() + vshift_nk = coreg.VerticalShift() + coreg.NuthKaab() + + nk_vshift.fit(**self.fit_params) + aligned_dem, _ = nk_vshift.apply(self.tba.data, transform=self.ref.transform, crs=self.ref.crs) + vshift_nk.fit(**self.fit_params) + aligned_dem, _ = vshift_nk.apply(self.tba.data, transform=self.ref.transform, crs=self.ref.crs) + + assert np.allclose(nk_vshift.to_matrix(), vshift_nk.to_matrix(), atol=10e-1) class TestBlockwiseCoreg: ref, tba, outlines = load_examples() # Load example reference, to-be-aligned and mask. From 15e92858dab2641263f7506c73313f878954d6c9 Mon Sep 17 00:00:00 2001 From: Romain Hugonnet Date: Mon, 27 Nov 2023 20:49:29 -0800 Subject: [PATCH 4/6] Linting --- tests/test_coreg/test_base.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_coreg/test_base.py b/tests/test_coreg/test_base.py index a6580107..0607c497 100644 --- a/tests/test_coreg/test_base.py +++ b/tests/test_coreg/test_base.py @@ -676,7 +676,7 @@ def test_coreg_add(self) -> None: vshift5 = vshift3 + vshift3 assert vshift5.to_matrix()[2, 3] == vshift * 4 - def test_pipeline_consistency(self): + def test_pipeline_consistency(self) -> None: """Check that pipelines properties are respected: reflectivity, fusion of same coreg""" # Test 1: Fusion of same coreg @@ -714,6 +714,7 @@ def test_pipeline_consistency(self): assert np.allclose(nk_vshift.to_matrix(), vshift_nk.to_matrix(), atol=10e-1) + class TestBlockwiseCoreg: ref, tba, outlines = load_examples() # Load example reference, to-be-aligned and mask. inlier_mask = ~outlines.create_mask(ref) From c7cb981034879cd440f769bc19706a6ae11460d4 Mon Sep 17 00:00:00 2001 From: Romain Hugonnet Date: Mon, 27 Nov 2023 21:29:26 -0800 Subject: [PATCH 5/6] Fix random states and increase tolerance --- tests/test_coreg/test_base.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/test_coreg/test_base.py b/tests/test_coreg/test_base.py index 0607c497..d7ff64ef 100644 --- a/tests/test_coreg/test_base.py +++ b/tests/test_coreg/test_base.py @@ -682,7 +682,7 @@ def test_pipeline_consistency(self) -> None: # Test 1: Fusion of same coreg # Many vertical shifts many_vshifts = coreg.VerticalShift() + coreg.VerticalShift() + coreg.VerticalShift() - many_vshifts.fit(**self.fit_params) + many_vshifts.fit(**self.fit_params, random_state=42) aligned_dem, _ = many_vshifts.apply(self.tba.data, transform=self.ref.transform, crs=self.ref.crs) # The last steps should have shifts of EXACTLY zero @@ -691,25 +691,25 @@ def test_pipeline_consistency(self) -> None: # Many horizontal + vertical shifts many_nks = coreg.NuthKaab() + coreg.NuthKaab() + coreg.NuthKaab() - many_nks.fit(**self.fit_params) + many_nks.fit(**self.fit_params, random_state=42) aligned_dem, _ = many_nks.apply(self.tba.data, transform=self.ref.transform, crs=self.ref.crs) # The last steps should have shifts of NEARLY zero - assert many_nks.pipeline[1]._meta["vshift"] == pytest.approx(0, abs=0.01) - assert many_nks.pipeline[1]._meta["offset_east_px"] == pytest.approx(0, abs=0.01) - assert many_nks.pipeline[1]._meta["offset_north_px"] == pytest.approx(0, abs=0.01) - assert many_nks.pipeline[2]._meta["vshift"] == pytest.approx(0, abs=0.01) - assert many_nks.pipeline[2]._meta["offset_east_px"] == pytest.approx(0, abs=0.01) - assert many_nks.pipeline[2]._meta["offset_north_px"] == pytest.approx(0, abs=0.01) + assert many_nks.pipeline[1]._meta["vshift"] == pytest.approx(0, abs=0.02) + assert many_nks.pipeline[1]._meta["offset_east_px"] == pytest.approx(0, abs=0.02) + assert many_nks.pipeline[1]._meta["offset_north_px"] == pytest.approx(0, abs=0.02) + assert many_nks.pipeline[2]._meta["vshift"] == pytest.approx(0, abs=0.02) + assert many_nks.pipeline[2]._meta["offset_east_px"] == pytest.approx(0, abs=0.02) + assert many_nks.pipeline[2]._meta["offset_north_px"] == pytest.approx(0, abs=0.02) # Test 2: Reflectivity # Those two pipelines should give almost the same result nk_vshift = coreg.NuthKaab() + coreg.VerticalShift() vshift_nk = coreg.VerticalShift() + coreg.NuthKaab() - nk_vshift.fit(**self.fit_params) + nk_vshift.fit(**self.fit_params, random_state=42) aligned_dem, _ = nk_vshift.apply(self.tba.data, transform=self.ref.transform, crs=self.ref.crs) - vshift_nk.fit(**self.fit_params) + vshift_nk.fit(**self.fit_params, random_state=42) aligned_dem, _ = vshift_nk.apply(self.tba.data, transform=self.ref.transform, crs=self.ref.crs) assert np.allclose(nk_vshift.to_matrix(), vshift_nk.to_matrix(), atol=10e-1) From 2b5b0d89d8c42431a9d1ace767849a8f67b06e8c Mon Sep 17 00:00:00 2001 From: Romain Hugonnet Date: Mon, 27 Nov 2023 21:29:53 -0800 Subject: [PATCH 6/6] Increase tolerance of tilt test --- tests/test_coreg/test_affine.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_coreg/test_affine.py b/tests/test_coreg/test_affine.py index c1c1fdd2..baf5e54e 100644 --- a/tests/test_coreg/test_affine.py +++ b/tests/test_coreg/test_affine.py @@ -288,7 +288,7 @@ def test_tilt(self) -> None: assert np.abs(np.mean(periglacial_offset)) < np.abs(np.mean(pre_offset)) # Check that the mean periglacial offset is low - assert np.abs(np.mean(periglacial_offset)) < 0.01 + assert np.abs(np.mean(periglacial_offset)) < 0.02 def test_icp_opencv(self) -> None: warnings.simplefilter("error")