-
Notifications
You must be signed in to change notification settings - Fork 47
Lack shape consistency on fit and apply #819
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
Lack shape consistency on fit and apply #819
Conversation
|
I have a problem that I don't understand why it isn't working. I would like to compare the NuthKaab.fit_and_apply() with :
I don't find the same output shifts and I don't know if it's normal or not. I don't think so but I don't understand why my additional code can lead to this error. |
|
Another strange problem : I don't find the same output shifts between :
Is that normal ? |
|
Note : need a validation for (E1) and (E2) @belletva @rhugonnet @adehecq |
|
@marinebcht Amazing description and summary, I grasped everything immediately. 😉 And great idea for the extra consistency tests! (crop/uncropped and pointcloud/raster) Here are possible explanations:
For the PR changes: |
|
Good idea but I would raise a warning and not an error. If I understand correctly, if About (E1), you confirm that we can't reproject a raster mask with ...
Plus, add |
|
Quick answers:
|
1/ Inlier mask out of boundsI don't know if we talk about the same thing but, to me, if a mask does not intersect the dem it currently make the fit_and_apply works like there is no mask, here a little test to explain it : => Intersection = False => Res stat Coreg with/without mask = True Plus, there is no actual test about the intersection (even with size ref_dem = size inlier_mask). 2/ ReprojectionOK ! Got it, thanks ! |
|
@marinebcht Yes indeed, the behaviour of 1/ is not correct, it would have to raise an error... I thought an error/warning was raised by inlier_mask_crop.reproject(reference_dem, resampling="nearest")Does not fail. It runs, but outputs a boolean raster full of 1s... I would have expected a Raster full of nodata values, but because nodata values don't exist for a boolean raster, they must be converted differently during the operation. We should probably add a new error/warning in |
|
Oh ! Ok so... maybe we can treat this particular case (inlier mask that not intersect input ref dem) in a open/new issues :
|
17d5513 to
39faf8f
Compare
e68a185 to
c040d4c
Compare
|
@marinebcht Following the meeting, I tried to understand the differences with cropped inlier_mask/ref/tba VS full inlier_mask/ref/tba you described above! To circumvent the current error in import geoutils as gu
import xdem
import numpy as np
# get data
reference_dem = xdem.DEM(xdem.examples.get_path("longyearbyen_ref_dem"))
dem_to_be_aligned = xdem.DEM(xdem.examples.get_path("longyearbyen_tba_dem"))
glacier_outlines = gu.Vector(xdem.examples.get_path("longyearbyen_glacier_outlines"))
inlier_mask = ~glacier_outlines.create_mask(reference_dem)
# crop all data (same size)
nrows, ncols = inlier_mask.shape
inlier_mask_crop = inlier_mask.icrop((0, 0, ncols - 100, nrows - 100))
reference_dem_crop = reference_dem.icrop((0, 0, ncols - 100, nrows - 100))
dem_to_be_aligned_crop = dem_to_be_aligned.icrop((0, 0, ncols - 100, nrows - 100))
# ADDED THIS STEP
inlier_mask_crop_ext = np.zeros(dem_to_be_aligned.shape, dtype=bool)
inlier_mask_crop_ext[0:-100, 0:-100] = inlier_mask_crop.data
list_shift = ["shift_x", "shift_y", "shift_z"]
# run large ref, large tba, cropped mask
nuthkaab_1 = xdem.coreg.NuthKaab(subsample=1)
nuthkaab_1.fit_and_apply(reference_dem, dem_to_be_aligned, inlier_mask=inlier_mask_crop_ext.data, random_state=42)
shifts_large_large_crop = [nuthkaab_1.meta["outputs"]["affine"][k] for k in list_shift]
print ("shift large ref, large tba, cropped mask:", shifts_large_large_crop)
# run cropped ref, cropped tba, cropped mask
nuthkaab_2 = xdem.coreg.NuthKaab(subsample=1)
nuthkaab_2.fit_and_apply(reference_dem_crop, dem_to_be_aligned_crop, inlier_mask=inlier_mask_crop.data, random_state=42)
shifts_crop_crop_crop = [nuthkaab_2.meta["outputs"]["affine"][k] for k in list_shift]
print ("shift cropped ref, cropped tba, cropped mask:", shifts_crop_crop_crop)Turning off Next, I'll look at the pointcloud/raster differences! |
|
Alright, I also figured out the differences for point-raster above! Some coregistration methods are "point-grid" and thus slightly assymetric because they use the grid to derive X/Y gradients to optimize the coregistration algorithms. This includes If we test a symmetric method, such as import xdem
reference_dem = xdem.DEM(xdem.examples.get_path("longyearbyen_ref_dem"))
reference_dem = reference_dem.icrop((0, 0, 100, 100))
reference_dem_pc = reference_dem.to_pointcloud().ds
reference_dem_pc.rename(columns={'b1': 'z'}, inplace=True)
dem_to_be_aligned = xdem.DEM(xdem.examples.get_path("longyearbyen_tba_dem"))
dem_to_be_aligned = dem_to_be_aligned.icrop((0, 0, 100, 100))
dem_to_be_aligned_pc = dem_to_be_aligned.to_pointcloud().ds
dem_to_be_aligned_pc.rename(columns={'b1': 'z'}, inplace=True)
list_shift = ["shift_x", "shift_y", "shift_z"]
# raster/raster
nuthkaab_1 = xdem.coreg.ICP(subsample=1, method="point-to-point")
nuthkaab_1.fit_and_apply(reference_dem, dem_to_be_aligned, random_state=42)
print([nuthkaab_1.meta["outputs"]["affine"][k] for k in list_shift])
# pc/raster
nuthkaab_2 = xdem.coreg.ICP(subsample=1, method="point-to-point")
nuthkaab_2.fit_and_apply(reference_dem_pc, dem_to_be_aligned, random_state=42)
print([nuthkaab_2.meta["outputs"]["affine"][k] for k in list_shift])
# raster/pc
nuthkaab_3 = xdem.coreg.ICP(subsample=1, method="point-to-point")
nuthkaab_3.fit_and_apply(reference_dem, dem_to_be_aligned_pc, random_state=42)
print([nuthkaab_3.meta["outputs"]["affine"][k] for k in list_shift])So all seems to be working as expected! 😄 |
|
Problem 1) Oh yes, I undestand now the consequence of the reprojection error in the case of a mask ^^ Top, I will add the test with the new geoutils branch/version ! Problem 2) Ok, I understand now even it was a little bit strange at first. Maybe we can add it somewhere in the doc ? (The fact that we can have different coreg results the a raster/raster vs raster/point cloud for several method) @belletva @adebardo |
|
Added #828 to add some tests for the two points I checked :
|
| - pytest-instafail | ||
| - pytest-socket | ||
| - pytest-cov | ||
| - coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file needs to be restored to match main (wrong merge?).
All these dependencies mirror the ones in setup.cfg, for those using a conda install.
| ("array", "pc", True, "array", "error", "Input mask array"), | ||
| ], | ||
| ) # type: ignore | ||
| def test_fit_and_apply__cropped_mask(self, combination: tuple[str, str, str, str, str, str]) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing series of tests, thanks a lot 😉
rhugonnet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect! See the small issue with the dev-env file.
We can add the new tests in a separate PR following your issue.
Resolves #756
Context
The problem appears when we need to do a fit_and_apply() with a different size mask of the elevation inputs.
As a reminder, the
fit_and_apply(reference_dem, dem_to_be_aligned)works like this :transform/crsinformationtransform/crsinformationtransform/crsinformationtransform/crsinformation(1*) error in
_preprocess_coreg_applyif not transform/crs(2*) /!\
warning f"'{name}' of type {type(dem)} overrides the given 'transform'")dans_preprocess_coreg_fit_raster_rastercarIf any input is a Raster, use its transform if 'transform is None'.(3*) error in
_preprocess_coreg_fitif not transform/crsResolution
When the mask is not the same size as elevation data, here are the processes :
mask is a raster
(a) _preprocess_coreg_fit_raster_raster
(b) _preprocess_coreg_fit_raster_point
mask is an array
(E2) = No solution found to reproject mask array
=> Raises error "Input mask array can't be a different size array as input elevation."
Tests
The reference results (dem + shift values) are computed with a cropped mask, reprojected with reference_dem.
All
nuth_kaab_crop.fit_and_applywith the different configs listed in the previous table + the cropped maskinlier_mask_crop_projneed to match the reference results.