Skip to content

ifg_inv: bugfix for design matrix w/ date1 > date2 & release memory occupied by covariance matrix#750

Merged
yunjunz merged 13 commits intoinsarlab:mainfrom
mirzaees:ifgram_inversion_bug
Mar 8, 2022
Merged

ifg_inv: bugfix for design matrix w/ date1 > date2 & release memory occupied by covariance matrix#750
yunjunz merged 13 commits intoinsarlab:mainfrom
mirzaees:ifgram_inversion_bug

Conversation

@mirzaees
Copy link
Collaborator

@mirzaees mirzaees commented Mar 6, 2022

Description of proposed changes

  1. There is a bug in the calculation of design matrix for ifgram inversion using minimum norm phase velocity. Currently the matrix only works for a network with sequential interferograms where the secondary images are always after reference date. So basically it does not work for some networks, for example a single reference network with the reference image in the middle. Incorrect timeseries are resulted where they are zero for dates before reference. Also weighting does not work in that case.

Proposed changes:

  • Correct design matrix calculation
  • Correct reference date based on interferograms
  • Remove the condition where rules out weighted inversion for non redundant network
  1. The second issue is regarding the memory and failure of Dask during inversion. The default of ifgram inversion is to not calculate/write covariance matrix of the timeseries which is a huge 4D matrix. But the problem is that it allocates memory for covariance matrix during inversion even when you don't want to calculate it

proposed changes:

  • Release memory occupied by covariance matrix if it is not opted to calculate for.

@mirzaees mirzaees requested review from hfattahi and yunjunz March 6, 2022 14:17
@yunjunz
Copy link
Member

yunjunz commented Mar 7, 2022

Thank you @mirzaees for the fixing, it looks great! I only have two minor comments above that I hope you could add to the code.

@yunjunz yunjunz changed the title Correct design matrix and release memory occupied by covariance matrix ifg_inv: bugfix for design matrix w/ date1 > date2 & release memory occupied by covariance matrix Mar 7, 2022
@mirzaees
Copy link
Collaborator Author

mirzaees commented Mar 8, 2022

@yunjunz Thanks for reviewing, your requested changes are committed

Copy link
Collaborator

@hfattahi hfattahi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mirzaees it looks good to me as far as I can tell. We don't have unit tests in place and I have not run your PR on any dataset. Since the PR touches core inversion module, before approving, could you or @yunjunz test and confirm that the regular workflows are not broken anywhere.

@mirzaees
Copy link
Collaborator Author

mirzaees commented Mar 8, 2022

@hfattahi Thank you, I have tested before committing and it works for me but it would be great if @yunjunz can test as well.

Copy link
Member

@yunjunz yunjunz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks all good to me. Thank you @mirzaees.

@yunjunz yunjunz merged commit 6e39b67 into insarlab:main Mar 8, 2022
@mirzaees mirzaees deleted the ifgram_inversion_bug branch June 29, 2023 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants