Multiple coreg fit_pts fixes.

[erikmannerfelt]

My first PR in quite some time!

Two things have been done in this PR; re-implementing my PR (liuh886#7) to the fork that was #346, and solving some small issues that I've found along the way.

The most important change: smashing the half-pixel shift inconsistency

The first part is arguably the most important because the fit_pts functions of NuthKaab and GradientDescending have been at risk of being half a pixel off since #346 was merged! The tests introduced in the PR did not actually check for the fit_pts result on points, but only on rasters converted to points in a predefined manner. My PR (liuh886#7) that fixed this was unfortunately lost along the way in #346... Anyway, here it is again, which fixes and tests the fit_pts half-pixel problem. The reason for the issue was that sampling was not done consistently in the centers of the pixels. The solution is quite hacky I feel, because I'm shifting coordinates and then sampling corners, but it works, as proven by the tests. Paging @liuh886 and @mmazzolini just to let you know that this problem exists in the current release of xdem. EDIT: It might be that the functionality worked before, but it wasn't tested properly, and could be half a pixel off with the wrong GDAL AREA_OR_POINT settings. This PR makes it so this won't happen (at least according to the tests).

Improvements

• The fit_pts functionality can now use a geometry column if available instead of E/N columns (d81ceb5)
• Added ICP fit_pts functionality (Revamp and simplify the raster-point/P-R/P-P functionality #402) (fixed in 9f28eeb).

Other fixes

• Solves Make CoregPipeline work with point data #406. Pipelines now work with point data (I added a small rudimentary test for it) (fixed in fd4fde3)
• Solves Coreg.fit_pts does not respect the z_name kwarg. #401 (fixed in d81ceb5)
• Solves Argument descriptions are missing in Coreg.fit_pts docstring. #403 (fixed in 4305eae)

[liuh886]

brilliant! Thanks, Eric, I am on the way back to Oslo, and I am going to have a test on the half-pixel shift bug next week!

[erikmannerfelt]

@adehecq, I had to disable your test (tests/test_coreg.py::test_dem_coregistration) because it segfaults both on my machine and in CI! Do you have any idea of why?

Since it shouldn't be due to this PR, I'll leave this test as skipped and then add an issue for it.

[erikmannerfelt]

Oh and @rhugonnet, I acknowledge that the ICP improvement efforts could be seen as misplaced to do now if we're to revamp everything either way. But this way, even the current functionality of xdem has more features. And more importantly for me, I can do my work like I intended now that ICP and CoregPipeline works with ICESat-2!

[rhugonnet]

Looking good, thanks for all the fixes and the tests!

I agree, we can leave the big refactoring to a separate PR, in particular:

• Use a single fit() method,
• Define "raster-raster", "point-raster" and "point-point" logic,
• Consistent point preprocessing/postprocessing.

Very weird to suddenly have a segmentation fault on dem_coregistration, but nowhere else (especially when it is just a wrapper of other functions)... 🤔

[erikmannerfelt]

Thanks for the inputs, @rhugonnet !

[adehecq]

@adehecq, I had to disable your test (tests/test_coreg.py::test_dem_coregistration) because it segfaults both on my machine and in CI! Do you have any idea of why?

Weird... Did you add an issue for this? I can have a look at it later, once we work on the big coreg changes.