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

Allow Raster (sub-)classes as inputs for Coreg.fit() and Coreg.apply() #174

Closed
erikmannerfelt opened this issue Jul 3, 2021 · 4 comments · Fixed by #175
Closed

Allow Raster (sub-)classes as inputs for Coreg.fit() and Coreg.apply() #174

erikmannerfelt opened this issue Jul 3, 2021 · 4 comments · Fixed by #175
Assignees
Labels
enhancement Feature improvement or request

Comments

@erikmannerfelt
Copy link
Contributor

We want to do two things: make things simple, and make them right. Allowing numpy arrays for DEMs is definitely right, as this can be used with rasterio and other frameworks. The simple part, however could maybe be improved by allowing Raster (sub-)classes as inputs for coregistration.

Data

>>> import xdem
>>> import geoutils as gu
>>> ref_dem = xdem.DEM(xdem.examples.get_path("longyearbyen_ref_dem"))
>>> tba_dem = xdem.DEM(xdem.examples.get_path("longyearbyen_tba_dem")).reproject(ref_dem)  # Reprojection would technically only be required in the "current syntax"
>>> glaciers = gu.Vector(xdem.examples.get_path("glacier_outlines"))

The current syntax

If one wants to coregister a DEM and save it, these are the steps that are involved:

>>> coreg = xdem.coreg.NuthKaab()
>>> coreg.fit(ref_dem.data, tba_dem.data, inlier_mask=~glaciers.create_mask(ref_dem), transform=ref_dem.transform)
>>> aligned_dem_data = coreg.apply(tba_dem.data, transform=tba_dem.transform)
>>> aligned_dem = xdem.DEM.from_array(aligned_dem_data, ref_dem.transform, ref_dem.crs)
>>> aligned_dem.save("path/to/dem_coreg.tif")

This is too long!!

Some issues I have with it

  • The .fit() method needs to be called in a separate line (but this will be fixed in Make 'Coreg.fit' return 'self' for one-liner coregistration. #173)
  • Coreg.fit() requires the transform argument, which could just as well have been read from the ref_dem raster.
  • Coreg.apply() returns a numpy array, which needs to be converted to a DEM/Raster to save.

Suggestions

  • A Raster (sub-)class should be possible to provide Coreg.fit(), whereby the transform argument should not be needed. A nearest neighbor reprojection could also be made to validate that they are on the same grid.
  • If a Raster (sub-)class is given to Coreg.apply(), it should also return a Raster (sub-)class of the same type.

Potential future syntax

>>> coreg = xdem.coreg.NuthKaab().fit(ref_dem, tba_dem, inlier_mask=~glaciers.create_mask(ref_dem))
>>> aligned_dem = coreg.apply(tba_dem)
>>> aligned_dem.save("path/to/dem_coreg.tif")

or in a horrible-to-debug one-liner!

>>> xdem.coreg.NuthKaab().fit(ref_dem,  tba_dem, inlier_mask=~glaciers.create_mask(ref_dem)).apply(tba_dem).save("path/to/dem_coreg.tif")

Of course, this new functionality should not change how numpy arrays are used, and the rasterio compatibility would not change.

@erikmannerfelt
Copy link
Contributor Author

The proposed syntax would work with #173 and #175!

@rhugonnet
Copy link
Contributor

or in a horrible-to-debug one-liner!

>>> xdem.coreg.NuthKaab().fit(ref_dem,  tba_dem, inlier_mask=~glaciers.create_mask(ref_dem)).apply(tba_dem).save("path/to/dem_coreg.tif")

I'm sorry what

@rhugonnet
Copy link
Contributor

Potential future syntax

>>> coreg = xdem.coreg.NuthKaab().fit(ref_dem, tba_dem, inlier_mask=~glaciers.create_mask(ref_dem))
>>> aligned_dem = coreg.apply(tba_dem)
>>> aligned_dem.save("path/to/dem_coreg.tif")

Yeah that's the dream

@adehecq
Copy link
Member

adehecq commented Jul 27, 2021

I'm a bit late but... I fully agree !! 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature improvement or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants