Add Option to Disable Vertical Shift in Nuth and Kääb Coregistration

[vschaffn]

Resolves #686.

Summary

This PR introduces a new feature to the Nuth and Kääb (2011) coregistration method, allowing users to optionally disable the vertical shift during the coregistration process. A new boolean parameter apply_vertical_shift has been added to control whether the vertical translation (shift in the z-axis) should be applied.

Key Changes

• Added apply_vertical_shift to the constructor of the NuthKaab class, defaulting to True to maintain the current behavior (vertical shift enabled by default).
• Modified method _nuth_kaab_iteration_step to conditionally apply the vertical shift based on the value of apply_vertical_shift.
  If the parameter is not provided, the default behavior (applying the vertical shift) remains unchanged, ensuring no impact on existing usage.

Usage example

coreg_method = xdem.coreg.NuthKaab(apply_vertical_shift=False)
coreg_method.fit(dem, dem_tba)
aligned_dem = coreg_method.apply(dem_tba)

in coreg_method.info():

[adebardo]

Could you please add some unit tests ?

[adebardo]

Hello @rhugonnet ,
Regarding the message left on the issue, during coregistration, the Z shift is not disabled; it is only disabled for the final result at the apply stage.

Does this ticket suit you as it is? (with unit tests added)

[vschaffn]

@adebardo I added a test to ensure that the vertical shift is 0 if apply_vertical_shift is False, and the horizontal shifts are the same with or without applying vertical shift.

[rhugonnet]

@adebardo You mean it's not disabled because we still remove the vertical median here?

xdem/xdem/coreg/affine.py

Line 476 in a147a5f

dh_step -= vshift

Hmm... by having the modification within the _nuth_kaab_iteration_step loop as done here, the resulting behaviour was difficult to grasp for me.

If the objective is simply not to apply a vertical shift, should we update this line instead directly in the NuthKaab class:

xdem/xdem/coreg/affine.py

Line 1397 in a147a5f

output_affine = OutAffineDict(shift_x=-easting_offset, shift_y=-northing_offset, shift_z=vertical_offset)

with shift_z=vertical_offset if apply_vertical_shift else 0.

The behaviour should be the same, and we wouldn't have to modify anything but this line.

[rhugonnet]

Or, I'm just realizing now, were you worried about the fact that the logging info would show a vertical shift at each iteration if the vertical shift is not modified within the function?
It'd be OK with me if a vertical shift value is shown during the iterations even for apply_vertical_shift=False, as it's simply about applying it!

[rhugonnet]

Ah and finally we should add the parameter apply_vertical_shift to the CoregDict parameters so that it shows in info() and such with other metadata (see: https://xdem.readthedocs.io/en/stable/coregistration.html#coregistration-metadata).

The steps are simply:

• Adding apply_vertical_shift here:

  xdem/xdem/coreg/base.py

  Line 1576 in a147a5f

  class InAffineDict(TypedDict, total=False):

• Adding a corresponding text here:

  xdem/xdem/coreg/base.py

  Line 92 in a147a5f

  dict_key_to_str = {

• And updating __init__ so that the parameter is passed to super() like the other ones 😉

(For the record, there's a wiki on coregistration with all steps described here: https://github.com/GlacioHack/xdem/wiki/Things-to-know-on-coregistration-class-structure#steps-to-add-a-new-coreg-subclass)

[vschaffn]

@rhugonnet Thank you for the instructions ! I added the parameter apply_vshift as a boolean to ther CoregDict parameters.

I don't know how to use github very well, sorry