search

GlacioHack / xdem

Analysis of digital elevation models (DEMs)
https://xdem.readthedocs.io/
MIT License
139 stars 39 forks source link

Coregistration: Tilt and VerticalShift pipelines in 0.0.16 #447

Closed MatteaE closed 10 months ago

MatteaE commented 10 months ago

I've encountered a regression with coregistration between 0.0.13 and 0.0.16, affecting (at least) Tilt and VerticalShift when used in combination with NuthKaab.

Attached the relevant dh plots: coregistration result is wrong in 0.0.16 with any combination of NuthKaab + one of the two methods mentioned above; when used individually, NuthKaab and VerticalShift behave as expected also in 0.0.16. Also attached the input files to reproduce.

demdiff_orig_plot

xdem_coreg_issue_data.zip

rhugonnet commented 10 months ago

Thanks a lot for the reproducible example @MatteaE! Probably a bug introduced in https://github.com/GlacioHack/xdem/pull/436. Our tests for the coregistration are a bit old (2 years) and didn't catch this... It's one of our top priorities to update them. I'll look at this in details later this week :slightly_smiling_face:

rhugonnet commented 10 months ago

Found the bug, 4 letters were missing in a variable of CoregPipeline.fit(). Added tests in #450 so that won't happen again :smile:. I'll release a 0.0.17 with the fix soon.

Thanks again @MatteaE!