-
Notifications
You must be signed in to change notification settings - Fork 67
Fix the same number of looks values in the GUNW metadata #149
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
Conversation
|
|
||
| if (unwrap_az_looks != 1) or (unwrap_rg_looks != 1): | ||
| unwrap_igram_range_looks = unwrap_rg_looks | ||
| unwrap_igram_azimuth_looks = unwrap_az_looks |
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.
@xhuang-jpl, we briefly reviewed this offline and you gave some context for this change (thank you!):
- The number of looks may differ between the wrapped product (RIFG) and the unwrapped product (RUNW).
- When the number of looks in the
phase_unwrapgroup of the runconfig is set to 1 (in both azimuth and range), then no additional looks are taken in the unwrapping step, so the number of looks in the unwrapped phase is the same as the number of looks in the wrapped interferogram. This is also documented in the runconfig:
isce3/share/nisar/defaults/insar.yaml
Lines 597 to 600 in 32642e4
| # Number of looks in slant range and azimuth directions to generate the | |
| # wrapped interferogram that will be unwrapped in RUNW. | |
| # If range_looks = 1 and azimuth_looks = 1 no further multilooking occurs | |
| # In this case the wrapped interferogram in RIFG will be unwrapped. |
Thanks for this clarification. I think the thing I'm still confused about is this:
If the number of looks in the phase_unwrap group of the runconfig is not 1, should it be the total number of looks, or the additional number of looks after multilooking by the number of looks specified in the crossmul group of the runconfig?
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.
Thank you @gmgunter, if the number of looks in the phase_unwrap is not 1, then it will be the total number of looks. The unwrapping and crossmul have independent number of looks.
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.
Yes I agree that it should be the total number of looks in that situation. The user would only care about the final number of looks and if looks are taken in one step or two steps is just implementation detail.
hfattahi
left a comment
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!
Tyler-g-hudson
left a comment
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
| unwrap_igram_range_looks = wrap_igram_range_looks | ||
| unwrap_igram_azimuth_looks = wrap_igram_azimuth_looks | ||
|
|
||
| if (unwrap_az_looks != 1) or (unwrap_rg_looks != 1): | ||
| unwrap_igram_range_looks = unwrap_rg_looks | ||
| unwrap_igram_azimuth_looks = unwrap_az_looks |
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.
| unwrap_igram_range_looks = wrap_igram_range_looks | |
| unwrap_igram_azimuth_looks = wrap_igram_azimuth_looks | |
| if (unwrap_az_looks != 1) or (unwrap_rg_looks != 1): | |
| unwrap_igram_range_looks = unwrap_rg_looks | |
| unwrap_igram_azimuth_looks = unwrap_az_looks | |
| if (unwrap_az_looks != 1) or (unwrap_rg_looks != 1): | |
| unwrap_igram_range_looks = unwrap_rg_looks | |
| unwrap_igram_azimuth_looks = unwrap_az_looks | |
| else: | |
| unwrap_igram_range_looks = wrap_igram_range_looks | |
| unwrap_igram_azimuth_looks = wrap_igram_azimuth_looks |
Just a little nitpick - these variables are always going to be assigned either way, this just organizes the assignments in an if/else block for fast reading of the logic
* update the number of looks * fix minors --------- Co-authored-by: Xiaodong Huang <xhuang@nisar-adt-dev-5.jpl.nasa.gov>
This PR is to fix the same number of looks values for the both wrapped and unwrapped interferogram in the GUNW metadata reported by Ian Joughin. The issue is raised because the GUNW writer class is inherent from both the RIFG and RUNW writer classes where those two variables
self.igram_range_looksandself.igram_azimuth_looksare overwritten when the functionadd_interferogram_to_procinfo_params_groupis called by one of the RIFG and RUNW writers.