Skip to content

Commit

Permalink
Fix phass unwrap (#240)
Browse files Browse the repository at this point in the history
* switch order of input/output arguments for PHASS

* remove dupe ovr creation

* always use `tophu` if not calling snaphu

PHASS is segfaulting. the inputs needs to be a float phase raster, not complex.

there doesnt seem to be a benefit to using isce3 over tophu

* allow dask install during test/docker

* need --no-deps for pip and isce3

* no deps in docker
  • Loading branch information
scottstanie authored Feb 27, 2024
1 parent 6e28294 commit c7a8695
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 29 deletions.
1 change: 1 addition & 0 deletions .github/workflows/test-build-push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ jobs:
- label: Latest
spec: >-
isce3
tophu
fail-fast: true
name: ${{ matrix.os }} • ${{ matrix.deps.label }}
Expand Down
39 changes: 22 additions & 17 deletions src/dolphin/unwrap/_isce3.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,17 +81,7 @@ def unwrap_isce3(
driver = "ENVI"
opts = list(io.DEFAULT_ENVI_OPTIONS)

if unwrap_method == UnwrapMethod.PHASS:
# TODO: expose the configuration for phass?
# If we ever find cases where changing help, then yes we should
# coherence_thresh: float = 0.2
# good_coherence: float = 0.7
# min_region_size: int = 200
unwrapper = Phass()
unw_nodata = -10_000
else:
unwrapper = ICU(buffer_lines=shape[0])
unw_nodata = 0
unw_nodata = -10_000 if unwrap_method == UnwrapMethod.PHASS else 0
# Create output rasters for unwrapped phase & connected component labels.
# Writing with `io.write_arr` because isce3 doesn't have creation options
io.write_arr(
Expand Down Expand Up @@ -133,13 +123,28 @@ def unwrap_isce3(
f"Unwrapping size {(ifg_raster.length, ifg_raster.width)} {ifg_filename} to"
f" {unw_filename} using {unwrap_method.value}"
)
if unwrap_method == UnwrapMethod.PHASS:
# TODO: expose the configuration for phass?
# If we ever find cases where changing help, then yes we should
# coherence_thresh: float = 0.2
# good_coherence: float = 0.7
# min_region_size: int = 200
unwrapper = Phass()
unwrapper.unwrap(
ifg_raster,
corr_raster,
unw_raster,
conncomp_raster,
)
else:
unwrapper = ICU(buffer_lines=shape[0])

unwrapper.unwrap(
unw_raster,
conncomp_raster,
ifg_raster,
corr_raster,
)
unwrapper.unwrap(
unw_raster,
conncomp_raster,
ifg_raster,
corr_raster,
)

del unw_raster, conncomp_raster
return Path(unw_filename), Path(conncomp_filename)
12 changes: 1 addition & 11 deletions src/dolphin/unwrap/_unwrap.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
DEFAULT_UNW_NODATA,
UNW_SUFFIX,
)
from ._isce3 import unwrap_isce3
from ._snaphu_py import unwrap_snaphu_py
from ._tophu import multiscale_unwrap
from ._utils import create_combined_mask, set_nodata_values
Expand Down Expand Up @@ -289,7 +288,7 @@ def unwrap(
cost=cost,
scratchdir=scratchdir,
)
elif any(t > 1 for t in ntiles):
else:
unw_path, conncomp_path = multiscale_unwrap(
ifg_filename,
corr_filename,
Expand All @@ -305,15 +304,6 @@ def unwrap(
scratchdir=scratchdir,
log_to_file=log_to_file,
)
else:
unw_path, conncomp_path = unwrap_isce3(
ifg_filename,
corr_filename,
unw_filename,
mask_file=combined_mask_file,
unwrap_method=unwrap_method,
zero_where_masked=zero_where_masked,
)

# TODO: post-processing steps go here:

Expand Down
1 change: 0 additions & 1 deletion src/dolphin/workflows/unwrapping.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ def run(
logger.info("Creating overviews for unwrapped images")
create_overviews(unwrapped_paths, image_type=ImageType.UNWRAPPED)
create_overviews(conncomp_paths, image_type=ImageType.CONNCOMP)
create_overviews(unwrapped_paths, image_type=ImageType.CORRELATION)

return (unwrapped_paths, conncomp_paths)

Expand Down

0 comments on commit c7a8695

Please sign in to comment.