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

[POC]: Add an initial shift for coregistration #603

Open
adebardo opened this issue Oct 15, 2024 · 1 comment · May be fixed by #650
Open

[POC]: Add an initial shift for coregistration #603

adebardo opened this issue Oct 15, 2024 · 1 comment · May be fixed by #650
Labels
[POC] Conception To review Tickets needs approval about it conception

Comments

@adebardo
Copy link

adebardo commented Oct 15, 2024

Context

The CNES wants to integrate the ability to instantiate an initial offset for the co-registration calculation. Because the dem_coregistration function allows for easy addition of this feature and we will recommend its use to CNES members, we propose simple modifications at this level.

Implementation

  1. Using the translate function from Geoutils:

    The translate function from Geoutils allows for translating a Digital Elevation Model (DEM). We will call this function in the dem_coregsitration method .

def dem_coregistration(
    src_dem_path: str | RasterType,
    ref_dem_path: str | RasterType,
    out_dem_path: str | None = None,
    coreg_method: Coreg | None = NuthKaab() + VerticalShift(),
    grid: str = "ref",
    resample: bool = False,
    resampling: rio.warp.Resampling | None = rio.warp.Resampling.bilinear,
    shp_list: list[str | gu.Vector] | tuple[str | gu.Vector] | tuple[()] = (),
    inout: list[int] | tuple[int] | tuple[()] = (),
    filtering: bool = True,
    dh_max: Number = None,
    nmad_factor: Number = 5,
    slope_lim: list[Number] | tuple[Number, Number] = (0.1, 40),
    plot: bool = False,
    out_fig: str = None,
+   estimated_initial_shift : list = [0, 0] 
) -> tuple[DEM, Coreg, pd.DataFrame, NDArrayf]:
+	"""
+	explain estimated_initial_shift + affine only
+	"""

	    if isinstance(ref_dem_path, str):
	        if grid == "ref":
	            ref_dem, src_dem = gu.raster.load_multiple_rasters([ref_dem_path, src_dem_path], ref_grid=0)
	        elif grid == "src":
	            ref_dem, src_dem = gu.raster.load_multiple_rasters([ref_dem_path, src_dem_path], ref_grid=1)
	    else:
	        ref_dem = ref_dem_path
	        src_dem = src_dem_path
	        if grid == "ref":
	            src_dem = src_dem.reproject(ref_dem, silent=True)
	        elif grid == "src":
	            ref_dem = ref_dem.reproject(src_dem, silent=True)
+      if estimated_initial_shift != [0, 0] :
+          logging.warning("Initial shift in affine mode only")
+          estimated_initial_shift = [estimated_initial_shift[0] * pixel_resolution_x, estimated_initial_shift[0] * pixel_resolution_y]
+          src_dem.translate(estimated_initial_shift[0], estimated_initial_shift[1], inplace=True)
	.
	.
	.
    # Coregister to reference - Note: this will spread NaN
    coreg_method.fit(ref_dem, src_dem, inlier_mask)
    dem_coreg = coreg_method.apply(src_dem, resample=resample, resampling=resampling)
+   if estimated_initial_shift != [0, 0] :
+   	calculated_shift_x = dem_coreg.meta["shift_x"]
+       calculated_shift_y = dem_coreg.meta["shift_y"]

+		dem_coreg.meta["shift_x"] = calculated_shift_x + estimated_initial_shift[0] 
+		dem_coreg.meta["shift_y"] = calculated_shift_x + estimated_initial_shift[1]

Tests

  • Set up a test for the dem_coregistration function with an initial offset value different from 0.
    Here, a new paragraph should be added to test this updated version.

Documentation

  • Update the documentation to include examples on using the new estimated_initial_shift

/estimation 3d

@adebardo adebardo added the [POC] Conception stuck The issue conception is stopped label Oct 15, 2024
@adebardo adebardo added [POC] Conception To review Tickets needs approval about it conception and removed [POC] Conception stuck The issue conception is stopped labels Oct 30, 2024
@rhugonnet
Copy link
Contributor

Looks good! 🙂

So the "explain affine only" would essentially be a check like coreg_method.is_affine for a Coreg object or all(c.is_affine for c in coreg_method) for a CoregPipeline object?
I'm not sure of the behaviour of the is_affine property on a pipeline right now (it might fail), we could modify it so it derives the proper attribute directly here:

def is_affine(self) -> bool:
(either by finding a solution for it to run on all subclasses, or simply by overridding the property in CoregPipeline).

@vschaffn vschaffn linked a pull request Nov 15, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[POC] Conception To review Tickets needs approval about it conception
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants