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

Improve CoregPipeline.apply() and .fit() performance by merging matrices #79

Open
erikmannerfelt opened this issue Apr 8, 2021 · 0 comments
Labels
performance Improvement to feature performance

Comments

@erikmannerfelt
Copy link
Contributor

If parts or all steps of a pipeline can be described as matrices, the current apply loop is inefficient and might induce resampling-related errors:

dem_mod = coreg.apply(dem_mod, transform)

A better approach would be to merge transformation matrices (when possible) and therefore do fewer resampling iterations, potentially lowering the error and the processing time.

A conceptual example is:

  1. A) BiasCorr (matrixable)
  2. B) ICP (matrixable)
  3. C) NuthKaab (matrixable)
  4. D) Deramp(degree=3) (not matrixable)

Currently, the pipeline would modify the DEM four times. By combining the matrices of A, B and C, only two modifications are needed:

  1. A-C
  2. D

This could be done by first implementing a property of Coreg, for example called is_rigid which is True if it can be represented as a rigid transform (4x4 matrix). Then, the following pseudocode could work:

dem_mod = dem.copy()

matrices: list[np.ndarray] = []
for coreg in self.pipeline:
    if coreg.is_rigid:
        # Append the matrix to the list of matrices to be applied at the same time.
        matrices.append(coreg.to_matrix())
    else:   # This marks when matrices can no longer be added, and need to be applied before the non-rigid apply is done
        # Use matrix-magic to combine the matrices
        combined_matrix = combine_matrix(matrices)
        # Use more matrix-magic to transform the DEM
        apply_matrix(dem_mod, combined_matrix)
        # Reset the matrices list
        matrices.clear()
        # Apply the non-rigid transform
        coreg.apply(dem_mod, transform=transform)

if len(matrices) > 0:
     # do the same as above to apply all the matrices

where the combine_matrix() and apply_matrix() functions need to be created.

@erikmannerfelt erikmannerfelt added the enhancement Feature improvement or request label Apr 8, 2021
@erikmannerfelt erikmannerfelt added performance Improvement to feature performance and removed enhancement Feature improvement or request labels Jul 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Improvement to feature performance
Projects
None yet
Development

No branches or pull requests

1 participant