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

non-closure phase bias estimation #765

Merged
merged 38 commits into from
Jul 27, 2022

Conversation

yjzhenglamarmota
Copy link
Contributor

@yjzhenglamarmota yjzhenglamarmota commented Apr 27, 2022

In this updated script,

I have added three action items:

  1. create_mask -- this retains the original portion of the script, creating a mask for areas susceptible to closure phase errors
  2. quick_biasEstimate -- this action item gives an approximate but fast estimate of bias time-series in a bw-n analysis (no inversion)
  3. biasEstimate -- this action item gives a more accurate estimate of bias time-series in a bw-n analysis (inversion involved)

-Yujie

@yunjunz
Copy link
Member

yunjunz commented Apr 27, 2022

@yjzhenglamarmota could you resolve the issues flagged by Codacy?

@yjzhenglamarmota
Copy link
Contributor Author

@yjzhenglamarmota could you resolve the issues flagged by Codacy?

Hi @yunjunz I have updated the code.

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.

Hi @yjzhenglamarmota , very nice work. I have not made it to the end yet. But here is few mostly minor comments. I will try to go over the rest soon.

mintpy/closure_phase_bias.py Outdated Show resolved Hide resolved
mintpy/closure_phase_bias.py Outdated Show resolved Hide resolved
mintpy/closure_phase_bias.py Outdated Show resolved Hide resolved
mintpy/closure_phase_bias.py Outdated Show resolved Hide resolved
mintpy/closure_phase_bias.py Outdated Show resolved Hide resolved
mintpy/closure_phase_bias.py Outdated Show resolved Hide resolved
mintpy/closure_phase_bias.py Outdated Show resolved Hide resolved
mintpy/closure_phase_bias.py Outdated Show resolved Hide resolved
mintpy/closure_phase_bias.py Outdated Show resolved Hide resolved
mintpy/closure_phase_bias.py Outdated Show resolved Hide resolved
mintpy/utils/isce_utils.py Outdated Show resolved Hide resolved
mintpy/utils/isce_utils.py Outdated Show resolved Hide resolved
@liwenhong123
Copy link

Can we use this error correction method for GAMMA or HYP3 unwarped graphs unwarped by MCF ?

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.

LGTM. Thanks @yjzhenglamarmota. This is a great addition to mintpy. Congrats on your nice work. Please make sure your notebook runs with the latest version of the code, before merging.

mintpy/closure_phase_bias.py Outdated Show resolved Hide resolved
mintpy/closure_phase_bias.py Outdated Show resolved Hide resolved
mintpy/closure_phase_bias.py Outdated Show resolved Hide resolved
@yjzhenglamarmota
Copy link
Contributor Author

Can we use this error correction method for GAMMA or HYP3 unwarped graphs unwarped by MCF ?

Hi @liwenhong123 , I am not so familiar with those. Currently, the only input the code requires is a stack of either wrapped or unwrapped interferograms in a format like ifgramStack.h5.

Reorganized the script to add in action items. Now can create mask with --action create_mask
Added function for quick and accurate bias-correction.
Debugged "action" bias_estimation
+ simplify sum_seq_closure_phase()
+ rename/simplify calc_closure_phase_mask()

+ break long lines
+ clean up string operations
+ stack.ifgramStack: add get_closure_phase_index()

+ refactor (sum_)seq_closure_phase():
   - use ifgramStack().get_closure_phase_index()
   - use array indexing to replace for loop for each closure loop
refactor the following two functions:

+ cum_seq_unw_closure_phase()
+ compute_unwrap_closure_phase()
+ fix a bug in sum_seq_closure_phase() while calling np.sum() introduced in refactoring

+ stack.py: add split2boxes()
+ closure_phase_bias.py:
   - add parallel processing for the correlation estimation and unwrapping of the closure phase via --num-worker option

   - remove the unnecessary inps.update_closure_phase option as it's now auto determined and skipped.

+ utils.isce_utils.py:
   - standardize the comments
   - merge Yunjun's private version of unwrap_snaphu() into here with the following changes:
   1. expose more options as func args
   2. add notes from Piyush on SNAPHU and Default configurations used in isce2 and FRINGE
   3. print out more useful snaphu settings and used time
   4. masked out wired values from SNAPHU in the unw file
   5. write metadata file for connected components.
+ improved auto-skip for unw seq closure phase
+ more print out msg for the bandwidth checking
+ light simplification for estimate_bias_timeseries_(patch)
@yunjunz
Copy link
Member

yunjunz commented Jul 21, 2022

Can we use this error correction method for GAMMA or HYP3 unwarped graphs unwarped by MCF ?

Hi @liwenhong123 , I am not so familiar with those. Currently, the only input the code requires is a stack of either wrapped or unwrapped interferograms in a format like ifgramStack.h5.

As @yjzhenglamarmota said above, the input is only the stack of unwrapped phase. So the products from Gamma and HyP3 are supported. But, we are currently relying on the isce2 module to filter and unwrap the closure phase interferograms, during the estimation, to mitigate the impact of phase unwrapping errors, as described in Zheng et al. (2022, TGRS), thus, isce2 is required in your mintpy python environment to run this script.

+ stack.ifgramStack: add get_sequential_closure_phase() from closure_phase_bias.py

+ closure_phase_bias.py:
   - use ifgram_inversion.estimate_timeseries() with scipy.linalg to replace np.linalg.pinv for 1) 2X speedup and 2) skip zero/nan values
   - add --water-mask option to skip pixels on the water body
   - merge seq_closure_phase() and sum_seq_closure_phase() into stack.ifgramStack.get_sequential_closure_phase()
   - more comments / notes from Zheng et al. (2022, TGRS)
   - add bandwidth2num_ifgram()
@yunjunz yunjunz changed the title Closure phase correction non-closure phase bias time-series estimation Jul 23, 2022
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.

This PR looks good to me for merging now (tests on one S1 stack with all 3 actions passed with reasonable results). Thank you @yjzhenglamarmota for this cool and important contribution!!

@yjzhenglamarmota As we discussed offline, could you update your notebook PR (insarlab/MintPy-tutorial#40) against the latest version of this PR to confirm everything works as expected? Then we could go ahead and merge this PR.

+ ifgram_inversion.py:
   - estimate_timeseries(): support inv_quality_name = 'no' to skip calculation of inversion quality
   - ifgram_inversion_patch(): use **kwargs to simplify the call to estimate_timeseries()

+ closure_phase_bias.py:
   - get_design_matrix_Wr(): adjust the Wr row/col order, for convention consistency
   - estimate_bias_timeseries_patch() speed up by:
   - 1. replace pixel-wise calc of wPhi_x with mat operation
   - 2. split TS estimation into mask_all_net and mask_par_net --> 2X speed up, similar to ifgram_inversion.py
@yunjunz yunjunz changed the title non-closure phase bias time-series estimation non-closure phase bias estimation Jul 23, 2022
+ for closure phase mask, use geometry file to mask out pixels with no-data-value

+ for quick_estimate, add dataset unit to the wratio.h5 file.
@yunjunz
Copy link
Member

yunjunz commented Jul 26, 2022

Pass testings with @yjzhenglamarmota offline. Merging this PR!

@yunjunz yunjunz merged commit 47c9bcc into insarlab:main Jul 27, 2022
@yjzhenglamarmota yjzhenglamarmota deleted the closure_phase_correction branch November 30, 2022 06:55
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.

4 participants