Add polarization-specific validDataMask datasets to InSAR product - #379
xhuang-jpl wants to merge 52 commits into
Conversation
|
Thanks for integrating the row-wise I ran this PR's head (
No regressions on this dataset. All 558 common datasets across the GUNW skeleton and the RIFG/RUNW scratch skeletons are byte-identical to the merge-base run except the usual run-varying metadata ( The performance and the tested-configuration results look good. Reading the integrated code turned up two validMask correctness notes in input configurations beyond the tested one, plus two minor cleanups — details and suggested fixes are folded below. Two validMask correctness notes (multi-frequency products; non-HH polarizations with a uint8 or missing inputDataExceptionMask) — worth addressing before merge; neither manifests on the frame above
Minor cleanups (fine as a follow-up)
On #359: since this PR carries the implementation, I'll downshift #359 to its regression tests only (the offer from the #358/#359 threads). The suite passes against this PR's head as-is — 11/11, including the adversarial rounding and bit-packing cases — so it can serve as the regression gate for the uint32 |
hfattahi
left a comment
There was a problem hiding this comment.
Very minor comments before we go to the details of this PR.
Also one note that we need to discuss in a PR review:
what is the behavior when the workflow runs on older/existing RSLCs with 8 bits inputDataExceptionMask. Ideally we should probably populate the InSAR validDataMask inside polarization group with the same subswathValidDataMask in "mask" dataset.
| ) | ||
| self._create_2d_dataset( | ||
| pixeloffsets_pol_group, | ||
| "validMask", |
There was a problem hiding this comment.
Since we already have subswath Valid Data Mask suggest
| "validMask", | |
| "validDataMask", |
| goff_shape, | ||
| np.uint8, | ||
| (f"Valid mask for the {pol} layers: " | ||
| "bit 1 = reference (1=valid, 0=invalid), bit 0 = secondary (1=valid, 0=invalid)"), |
There was a problem hiding this comment.
Can we expand and make the description more informative. Maybe something like this:
| "bit 1 = reference (1=valid, 0=invalid), bit 0 = secondary (1=valid, 0=invalid)"), | |
| "bit 1 = reference (1=valid, 0=invalid), bit 0 = secondary (1=valid, 0=invalid). Valid represents fully focused data and invalid represents partially focused or missing data"), |
@bhawkins please take a look if that extra description is consistent with RSLC.
| ( | ||
| "validMask", | ||
| np.uint8, | ||
| f"Valid mask for the {pol} layers: bit 1 = reference (1=valid, 0=invalid), bit 0 = secondary (1=valid, 0=invalid)", |
There was a problem hiding this comment.
Similar comment as above about more clear description.
bhawkins
left a comment
There was a problem hiding this comment.
Here's what we came up with looking though this today. However, we ran out of time, and I may need to revise some of these comments once I have a better understanding of what's going on here.
|
@hfattahi For bookkeeping: |
|
Thanks @s-sasaki-earthsea-wizard for finding those major bugs, which have been fixed. @hfattahi if it is old RSLC uint8, it will fallback to the validSubswathMask, but its values are only 1 or 0, not the subswath mask number. |
validMask datasets to InSAR productvalidDataMask datasets to InSAR product
…o fix_insar_mask
bhawkins
left a comment
There was a problem hiding this comment.
LGTM! Thanks for all your hard work on this patch.
|
@xhuang-jpl Preferably let's merge Jungkyo's PR first #381 as there seems to be minor conflict that you will need to resolve. |
hfattahi
left a comment
There was a problem hiding this comment.
LGTM! Thanks @xhuang-jpl for this great addition.
Summary
Add polarization-specific
validMaskdatasets to InSAR products for tracking reference and secondary image validity per polarization.Changes
validMask(uint8) with bit encoding - bit 1: reference validity, bit 0: secondary validityget_valid_mask_input_output()and updated CPU/GPU workflows to geocode validMask