-
Notifications
You must be signed in to change notification settings - Fork 263
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
non-closure phase bias estimation #765
Conversation
@yjzhenglamarmota could you resolve the issues flagged by Codacy? |
2b5eae3
to
2de1f03
Compare
Hi @yunjunz I have updated the code. |
There was a problem hiding this 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.
Can we use this error correction method for GAMMA or HYP3 unwarped graphs unwarped by MCF ? |
13dcc20
to
e013ab9
Compare
There was a problem hiding this 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.
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. |
f3a1167
to
9b8b221
Compare
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)
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()
There was a problem hiding this 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
+ 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.
Pass testings with @yjzhenglamarmota offline. Merging this PR! |
In this updated script,
I have added three action items:
-Yujie