Toggle zero mask and apply to unw phase field

[sssangha]

When passing a mask for unwrapping, it seems to be ignored even though an INT mask was clearly generated (see screenshot below):

After digging a bit, I realized that this was because by default the zero_where_masked parameter is set to False, which according to this conditional downstream must be set to true in addition to providing a mask.

In this PR, I've exposed this zero_where_masked parameter so it can be toggled. It seems to work now as here is the resulting UNW phase field:

That said, clearly snaphu-related artifacts appear to persist in the masked region, so I made some additions to the code to apply the mask to the final unw phase field which now looks as I would expect:

[scottstanie]

So I turned off the zero_where_masked option in this PR, as we're passing the mask to snaphu here:

dolphin/src/dolphin/unwrap/_snaphu_py.py

Line 126 in 2a5d0fa

mask=mask,

Snaphu does ignore the values in the mask (it sets the amplitudes to 0 while it's running).

I can't tell if the mask is getting ignored from your first image actually- the unwrapping in the island looks okay.. what does the connected component label file look like? If it's 0 around the water, that's what we'd want.
I don't think we want to set the output phase from an unwrapped interferogram to 0 everywhere that a water mask says... there's too many people who would object to certain water masks, so our plan was to reset the phase back to the wrapped phase, and have the output mask (i.e. the 0 connected component label) say "dont trust the phase here".

If we set the water to 0, there is no chance of somebody later re-wrapping and doing a better job unwrapping if they disagree with the mask

[sssangha]

So I turned off the zero_where_masked option in this PR, as we're passing the mask to snaphu here:

dolphin/src/dolphin/unwrap/_snaphu_py.py

Line 126 in 2a5d0fa

mask=mask,

Snaphu does ignore the values in the mask (it sets the amplitudes to 0 while it's running).

I can't tell if the mask is getting ignored from your first image actually- the unwrapping in the island looks okay.. what does the connected component label file look like? If it's 0 around the water, that's what we'd want. I don't think we want to set the output phase from an unwrapped interferogram to 0 everywhere that a water mask says... there's too many people who would object to certain water masks, so our plan was to reset the phase back to the wrapped phase, and have the output mask (i.e. the 0 connected component label) say "dont trust the phase here".

If we set the water to 0, there is no chance of somebody later re-wrapping and doing a better job unwrapping if they disagree with the mask

Yeah the conncomp mask does seem to capture this.

But I should clarify that the original unw field 20221005_20221029.unw.tif isn't overwritten, but rather a separate new 20221005_20221029.unw.zeroed.tif file is generated so whoever wants to use that can, which mirrors the convention we see for the INT fields (i.e. 20221005_20221029.int.tif vs 20221005_20221029.int.zeroed.tif). So do you think that this is something we can support and appropriate as I've proposed?

[scottstanie]

Oh sorry, i didn't see you also made it configurable now.

Yeah this does seem like a better way to do it rather than me decided for everyone haha.

[scottstanie]

Pending the stitching reverts

[sssangha]

Thanks @scottstanie !

Ok I made the requested edits, but I notice one of the checks is failing. I'm having a hard time to reconcile this:

src/dolphin/unwrap/_snaphu_py.py:149: error: Incompatible types in assignment (expression has type "Path", variable has type "str")  [assignment]
src/dolphin/unwrap/_tophu.py:187: error: Incompatible types in assignment (expression has type "Path", variable has type "str")  [assignment]
src/dolphin/unwrap/_isce3.py:150: error: Incompatible types in assignment (expression has type "Path", variable has type "str")  [assignment]

I thought I had called _zero_from_mask as intended, and it's working on my end. So I'm not sure what's the problem. Could you perhaps advise? Thanks

[scottstanie]

Thanks @scottstanie !

Ok I made the requested edits, but I notice one of the checks is failing. I'm having a hard time to reconcile this:

src/dolphin/unwrap/_snaphu_py.py:149: error: Incompatible types in assignment (expression has type "Path", variable has type "str")  [assignment]
src/dolphin/unwrap/_tophu.py:187: error: Incompatible types in assignment (expression has type "Path", variable has type "str")  [assignment]
src/dolphin/unwrap/_isce3.py:150: error: Incompatible types in assignment (expression has type "Path", variable has type "str")  [assignment]

I thought I had called _zero_from_mask as intended, and it's working on my end. So I'm not sure what's the problem. Could you perhaps advise? Thanks

mypy usually doesn't like when you overwrite/redefine a variable with a different type.
One possible fix would be something like
(at the end of snaphu-py)

        if zero_where_masked and mask_file is not None:
            logger.info("Zeroing unw/conncomp of pixels masked in " f"{mask_file}")
            return _zero_from_mask(unw_filename, cc_filename, mask_file)

    return Path(unw_filename), Path(cc_filename)

since the return type would be the same (also note that you dont need to coerce the unw_filename to a string to input it into _zero_from_mask)

[sssangha]

@scottstanie ok, I finally resolved the cli-related issues!

I also had to make some tweaks to _unwrap.py to return the masked unw phase fields if they were generated. Before, the masked fields were just generated, but then the unmasked unw phase fields were passed anyways.

Again though, I ensured this is only done if the two conditionals (zero_where_masked=True and a valid mask file is specified) are met.