xdem icon indicating copy to clipboard operation
xdem copied to clipboard

Re-organization of `coreg.py`

Open rhugonnet opened this issue 1 year ago • 3 comments

Several ideas here in relation to the one-liner coregistration: https://github.com/GlacioHack/xdem/pull/267#pullrequestreview-1145346346

More generically:

  • The core functionalities of Coreg methods should live outside of Coreg classes for readability and flexibility (e.g., functions to estimate the horizontal shift, to deramp, etc). In other words all Coreg methods should only be wrappers of already existing functions;
  • Optimization functions should call the methods of fit.py instead of directly calling scipy.optimize, so that their tolerance and outputs are consistently tested, and that any type of optimization method can be used;
  • While all class method should retain flexibility to work with np.ndarray, they should still be optimized to function with Raster objects (e.g., modifying the geotransform instead of resampling in case of a horizontal shift). To have a similar default behaviour for np.ndarray objects, we could potentially introduce a return of a transform object as output of apply().

Maybe @adehecq has more specific things to add!

rhugonnet avatar Oct 25 '22 09:10 rhugonnet

Yes, I agree with those points. Below I outlined the main steps to be done, hopefully in logical order:

  1. Step 2 requires that the data is convertible to a DEM instance -> Change coreg apply and fit functions so that they accept either an xdem.DEM instance, or a numpy array, transform and CRS.
  2. Replace all Coreg core functionalities with either standalone function or xdem/geoutils functions
    • Replace calculate_slope_and_aspect with xdem.terrain attributes
    • Replace subsampling with geoutils’ subsampling
    • Remove duplicates of deramping
  3. Add function to calculate standard stats before/after coregistration (mean, median, NMAD, std, others?). Call this function in apply.
  4. Add plotting step in all apply methods, and in some fit methods, like N&K.
  5. Update Coreg so that coregistration params are more easily output than currently with _meta
  6. Add function to create inlier mask (load vectors, remove slopes, outliers…)
  7. Save input/output DEM during fit and apply functions to avoid redoing calc?
  8. Remove unused/redundant functions (filter_by_range, filtered_slope,apply_xy_shift, apply_z_shift, mask_as_array, dilate_mask option, Coreg.residuals and error)

adehecq avatar Oct 25 '22 12:10 adehecq

Adding some points after closing #158: 9. Move apply_matrix from base.py to affine.py by overridding apply() in AffineCoreg (as done in BiasCorr), and related functions, 10. Add a run or fit_and_apply class method for Coreg, 11. Add a is_rigid property for AffineCoreg and subclasses, 12. Improve tests of apply_matrix or add a geo-ref based one? This joins #110. 13. Refine documentation pages to differentiate affine/non-affine (bias corrections) and explain the generic inputs/ouputs.

rhugonnet avatar Jul 29 '23 20:07 rhugonnet

Since #436, relevant for the re-org: Make subsampling consistent in AffineCoreg classes once the inside of _fit_func are consistently defined as functions working on 1D vector inputs (as in BiasCorr)

rhugonnet avatar Sep 25 '23 22:09 rhugonnet