GeoscienceAustralia / PyRate

A Python tool for estimating velocity and time-series from Interferometric Synthetic Aperture Radar (InSAR) data.
https://geoscienceaustralia.github.io/PyRate/
Apache License 2.0
200 stars 70 forks source link

Fix orbital corrections #303

Closed basaks closed 3 years ago

basaks commented 3 years ago

@mcgarth 23 July 2021: This PR implements significant changes to the orbital module as follows:

@basaks original description of the PR (titled Add multiprocessing and multilooking capability to independent orbital correction) as follows:

  1. This PR add multilooking capability during independent orbital correction.
  2. This PR also adds multiprocessing to independent orbital correction method.

It uses a similar least squares solution, except the model is based on only the interferogram's own multilooked phase data. The usefulness of this multilooking while using independent method should be compared against:

The independent method is trivially parallel and memory issues are not a concern. Using multilooking during independent correction requies the following additional steps:

  1. Create a in-memory (or on disc) multilooked file. We create the multilooked file in memory and assumed that it will be faster than creating it on disc.
  2. Create an additonal deisgn matrix same size as that of the original ifg.
  3. Too many nans in any one of the ifgs may make the process (least square solution) unstable.

Now the implementation is available in pyrate in case someone finds an use.

mcgarth commented 3 years ago

@mcgarth and @chandra2ga will review this one

richardt94 commented 3 years ago

It's not clear to me from the further commits/discussion since November last year whether @tfuhrmann's observations have been explained/resolved. Does the removal of the scale factor from the design matrix fix the issue with the multilooked and multiprocessed independent correction?

mcgarth commented 3 years ago

It's not clear to me from the further commits/discussion since November last year whether @tfuhrmann's observations have been explained/resolved. Does the removal of the scale factor from the design matrix fix the issue with the multilooked and multiprocessed independent correction?

To me, @tfuhrmann's observations show that there is a change to independent method results when using multi-looking >1. This is expected based on the scope of the PR (to enable multi-looking for the independent method). I have not fully assessed whether @tfuhrmann/@chandra2ga's observations are remedied by the latest changes to the PR. I think a full comparison/validation of independent and network methods is needed, but not in scope for approving this PR. Your thoughts @richardt94 ?

richardt94 commented 3 years ago

I think we shouldn't necessarily expect the methods to agree with each other, as they're using different input data, but we definitely need tests to confirm that they're working as expected. In particular - the orbital correction is fitting a linear, quadratic or cubic function to the data. Let's make some synthetic ifgs which are just linear/quadratic/cubic orbital errors plus some noise, and incorporate these in the test suite to confirm that all the orbit correction methods recover approximately correct orbital correction values.

richardt94 commented 3 years ago

I have separated out the logic which calculates the networked orbital correction from the main network correction function, which has some undesirable file I/O stuff going on in it and really all we want to do is test that the logic successfully detects a function of the right form in a network of interferograms - we don't really care about checking that the interferograms can be saved afterwards because this is presumably elsewhere in the test suite.

This test passes for the example network I gave it (and I'm waiting on the full CI suite to check I didn't introduce any regressions). I will work on a similar test where I multilook the ifgs first and check that it still works, as the normal use case for the network orbital correction will be multilooked, and then revisit the independent method tests to see if they can be improved.

Before merging any of this, I'm a little worried that the design matrix appears to assign the constant coefficient to each interferogram rather than each epoch for the network inversion - is this intentional? It seems to me that the network method should be able to recover a model orbit_error = a*x + b*y + c, where the coefficients a, b, c are defined per-epoch.

richardt94 commented 3 years ago

I have implemented the network orbital correction using a constant parameter for each epoch rather than each interferogram, as discussed in my previous comment. This breaks some tests which compare against a reference implementation of the "old" method - I have skipped these. We should discuss whether this is the method we want to go with before merging the PR - in particular, the "old" and "new" network methods will interact differently with the reference pixel subtraction, which happens before any of the orbital corrections.