Skip to content

Add interpolation option for unwrapping#264

Merged
mirzaees merged 19 commits into
isce-framework:mainfrom
mirzaees:interpolation_for_unwrap
Mar 19, 2024
Merged

Add interpolation option for unwrapping#264
mirzaees merged 19 commits into
isce-framework:mainfrom
mirzaees:interpolation_for_unwrap

Conversation

@mirzaees
Copy link
Copy Markdown
Collaborator

@mirzaees mirzaees commented Mar 12, 2024

  • Added interpolation function for masked pixels before unwrapping to ease the computations
  • Added unit test and changed sample interferograms to a simple ramp instead of all zero

@mirzaees mirzaees requested a review from scottstanie March 12, 2024 22:39
Copy link
Copy Markdown
Collaborator

@hfattahi hfattahi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mirzaees for this useful addition. I have few comments below. Also if I understand correctly, it looks like the current implementation allows to either use run_goldstein or run_interpolation or neither of them. I don't see a functionality to run both. Is that correct?

Comment thread src/dolphin/interpolation.py Outdated
Comment thread src/dolphin/unwrap/_unwrap.py Outdated
Comment thread src/dolphin/interpolation.py
Comment thread src/dolphin/interpolation.py
Copy link
Copy Markdown
Collaborator

@hfattahi hfattahi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing my comments. My final comment is that the current unit test does not really check if the interpolation function does the right thing or not. Would it be possible to add my suggested test below?

Comment thread src/dolphin/interpolation.py
@mirzaees
Copy link
Copy Markdown
Collaborator Author

Thanks for addressing my comments. My final comment is that the current unit test does not really check if the interpolation function does the right thing or not. Would it be possible to add my suggested test below?

Thanks @hfattahi for your suggestion, I added a testing function for that. Also changed the sample interferograms a bit to be closer to reality. the sample interferograms were all zero

Comment thread tests/test_unwrap.py Outdated
Copy link
Copy Markdown
Collaborator

@hfattahi hfattahi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see below for my suggestion to make the test more clear. Please feel free to change the detailed implementation. I just meant to give a sudo code to describe the idea. In general please be more generous in commenting the code so that we can always get back to them and understand what we have done or what were our assumptions/algorithms etc. We are very close. Thanks for your patience and addressing my comments.

Comment thread tests/test_unwrap.py Outdated
Comment thread tests/test_unwrap.py Outdated
Copy link
Copy Markdown
Collaborator

@hfattahi hfattahi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks @mirzaees and thanks to @taliboliver and @scottstanie for their initial work on this.

@mirzaees mirzaees merged commit 984d42e into isce-framework:main Mar 19, 2024
scottstanie added a commit to scottstanie/dolphin that referenced this pull request May 17, 2024
* add interpolation option for unwrapping

* add reference, convert float weights once

* remove the extra all check

* Remove pymp reference

* remove todo

* Pass through the correlation threshold for interpolation

The last implementation had a default of 0.0
Since every weight was >=0, no pixels were interpolated.

* support to have both interpolation and filtering

* change the values of all zero sample interferogram

* add test for interpolation loop

* change docstring to match datatype

---------

Co-authored-by: mirzaees <smirzaee@aurora.jpl.nasa.gov>
Co-authored-by: Scott Staniewicz <scott.j.staniewicz@jpl.nasa.gov>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
scottstanie added a commit that referenced this pull request Jun 21, 2024
* start paper folder

* add skele

* fix names

* add fig placeholder, new ref

* fix makefile

* fix paper instructions

* add ref

* more refs

* refs

* draft to end

* add paper action

* missing space

* no need to run full test for this branch

* typo

* fix ref

* +

* Enforce consistency between jax and jaxlib (#252)

Consistency between jax and jaxlib can be enforced by installing the most recent versions via pip, which is currently not possible through conda where the two packages are out of sync.

This is necessary as dolphin would crash early on in processing if the two packages are not in sync.

* Disable corrections part of pytest, add one for timeseries (#254)

* dont exit before unwrapping for full test

* make test data have mostly signal, not pure noise

* fix the failing test due to len 1 stack losing dimension

* raise an error if the reference point selection fails

* fix 3d writing, pytest gets farther

* `max_bandwidth` can be 1: fix `gt` pydantic restriction

* try more

* disable corrections pytest

This is very annoying to get working with fake data.
The current error is:
RuntimeError: The -tr option was used, but there's no geotransform or it is
rotated.  This configuration is not supported.

The geometdata for statis layers is wrong

* fix jax warning

* [skip ci] fix changelog date

* Add option to toggle off inversion (#255)

* Add option to toggle off inversion

Add option to toggle inversion in the config function. By default set to true, like for unwrapping.

This allows for more flexibility when running especially large-scale processing.

* Make downstream updates to displacement.py

* Revert outdated disp changes

* Revert another vestigial change to displacement

* Add `similarity` module (#256)

* add similarity module and `median_similarity` funciton

* extract `process_blocks` to an io module

* add basic tests

* add max sim, add process blocks function

* fix argument ordering for `invert`

* fix block proc ordering and protocol definitions

* fix process check, zip self similarity check

* make strictly less

* remove `-tap` option from `gdal_merge.py` call (#257)

* Goldstein filtering step for unwrapping workflow (#247)

* Implement goldstein filtering step for unwrapping workflow

* Fix linter checks, reformat

* Default goldstein filtering to false

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Fix mypy/ruff issues, add type annotations

* Fix numpy typing annotation

* Add goldstein pytest

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* [pre-commit.ci] pre-commit autoupdate (#263)

* [pre-commit.ci] pre-commit autoupdate

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.2.2 → v0.3.2](astral-sh/ruff-pre-commit@v0.2.2...v0.3.2)
- [github.com/pre-commit/mirrors-mypy: v1.8.0 → v1.9.0](pre-commit/mirrors-mypy@v1.8.0...v1.9.0)

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Create dependabot.yml (#266)

* Bump the actions group with 7 updates (#267)

* Bump the actions group with 7 updates

Bumps the actions group with 7 updates:

| Package | From | To |
| --- | --- | --- |
| [actions/checkout](https://github.com/actions/checkout) | `3` | `4` |
| [actions/cache](https://github.com/actions/cache) | `3` | `4` |
| [actions/upload-artifact](https://github.com/actions/upload-artifact) | `3` | `4` |
| [actions/setup-python](https://github.com/actions/setup-python) | `4` | `5` |
| [docker/login-action](https://github.com/docker/login-action) | `2` | `3` |
| [docker/build-push-action](https://github.com/docker/build-push-action) | `4` | `5` |
| [akhilerm/tag-push-action](https://github.com/akhilerm/tag-push-action) | `2.1.0` | `2.2.0` |


Updates `actions/checkout` from 3 to 4
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v3...v4)

Updates `actions/cache` from 3 to 4
- [Release notes](https://github.com/actions/cache/releases)
- [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md)
- [Commits](actions/cache@v3...v4)

Updates `actions/upload-artifact` from 3 to 4
- [Release notes](https://github.com/actions/upload-artifact/releases)
- [Commits](actions/upload-artifact@v3...v4)

Updates `actions/setup-python` from 4 to 5
- [Release notes](https://github.com/actions/setup-python/releases)
- [Commits](actions/setup-python@v4...v5)

Updates `docker/login-action` from 2 to 3
- [Release notes](https://github.com/docker/login-action/releases)
- [Commits](docker/login-action@v2...v3)

Updates `docker/build-push-action` from 4 to 5
- [Release notes](https://github.com/docker/build-push-action/releases)
- [Commits](docker/build-push-action@v4...v5)

Updates `akhilerm/tag-push-action` from 2.1.0 to 2.2.0
- [Release notes](https://github.com/akhilerm/tag-push-action/releases)
- [Commits](akhilerm/tag-push-action@v2.1.0...v2.2.0)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: actions
- dependency-name: actions/cache
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: actions
- dependency-name: actions/upload-artifact
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: actions
- dependency-name: actions/setup-python
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: actions
- dependency-name: docker/login-action
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: actions
- dependency-name: docker/build-push-action
  dependency-type: direct:production
  update-type: version-update:semver-major
  dependency-group: actions
- dependency-name: akhilerm/tag-push-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: actions
...

Signed-off-by: dependabot[bot] <support@github.com>

* add write package permission

see https://docs.github.com/en/actions/publishing-packages/publishing-docker-images#publishing-images-to-github-packages

also https://stackoverflow.com/questions/76607955/error-denied-installation-not-allowed-to-create-organization-package

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Scott Staniewicz <scott.j.staniewicz@jpl.nasa.gov>

* Toggle zero mask and apply to unw phase field (#262)

* Add option to toggle zeromsk for unw

* Add unwrapping mask

* snaphu ci string fix

* tophu ci str fix

* remove typo from common

* Pass zero unw files if applicable

* Fix snaphu cli error

---------

Co-authored-by: Simran S Sangha <ssangha@trappist.jpl.nasa.gov>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* [pre-commit.ci] pre-commit autoupdate (#270)

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.3.2 → v0.3.3](astral-sh/ruff-pre-commit@v0.3.2...v0.3.3)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Add interpolation option for unwrapping (#264)

* add interpolation option for unwrapping

* add reference, convert float weights once

* remove the extra all check

* Remove pymp reference

* remove todo

* Pass through the correlation threshold for interpolation

The last implementation had a default of 0.0
Since every weight was >=0, no pixels were interpolated.

* support to have both interpolation and filtering

* change the values of all zero sample interferogram

* add test for interpolation loop

* change docstring to match datatype

---------

Co-authored-by: mirzaees <smirzaee@aurora.jpl.nasa.gov>
Co-authored-by: Scott Staniewicz <scott.j.staniewicz@jpl.nasa.gov>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* [pre-commit.ci] pre-commit autoupdate (#271)

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.3.3 → v0.3.4](astral-sh/ruff-pre-commit@v0.3.3...v0.3.4)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Grow conncomp (#272)

* add grow conncomp

* change typing

* select reference point with a general condition file

* Update src/dolphin/timeseries.py

* use a more generic form of Callable

* use with to open files

---------

Co-authored-by: mirzaees <smirzaee@aurora.jpl.nasa.gov>
Co-authored-by: Geoffrey Gunter <12984092+gmgunter@users.noreply.github.com>

* [pre-commit.ci] pre-commit autoupdate (#273)

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.3.4 → v0.3.5](astral-sh/ruff-pre-commit@v0.3.4...v0.3.5)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* [pre-commit.ci] pre-commit autoupdate (#275)

updates:
- [github.com/pre-commit/pre-commit-hooks: v4.5.0 → v4.6.0](pre-commit/pre-commit-hooks@v4.5.0...v4.6.0)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Compressed slc naming (#274)

* change compressed slc naming in unit tests
* change default for tilecostthreshold

---------

Co-authored-by: mirzaees <smirzaee@aurora.jpl.nasa.gov>
Co-authored-by: Scott Staniewicz <scott.j.staniewicz@jpl.nasa.gov>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Docs cleanup (#268)

* use `pymdownx.snippets` to cut repeated docs from readme. add `CONTRIBUTING.MD`

* remove the extra gpu env file since Jax needs separate installs

* Revert #252

* update contrib, setup, tutorial map

* add JAX GPU setup notes

* typos

* update the change log (#276)

* update the change log

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: mirzaees <smirzaee@aurora.jpl.nasa.gov>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* [pre-commit.ci] pre-commit autoupdate (#278)

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.3.5 → v0.3.7](astral-sh/ruff-pre-commit@v0.3.5...v0.3.7)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Cli timeseries (#279)

* add timeseries step to cli

* pre-commit fix

---------

Co-authored-by: mirzaees <smirzaee@aurora.jpl.nasa.gov>

* [pre-commit.ci] pre-commit autoupdate (#280)

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.3.7 → v0.4.1](astral-sh/ruff-pre-commit@v0.3.7...v0.4.1)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* correct reference point typing (#281)

* correct reference point typing

* correct coherence threshold typing

* correct raising error

---------

Co-authored-by: mirzaees <smirzaee@aurora.jpl.nasa.gov>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* [pre-commit.ci] pre-commit autoupdate (#283)

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.4.1 → v0.4.2](astral-sh/ruff-pre-commit@v0.4.1...v0.4.2)
- [github.com/pre-commit/mirrors-mypy: v1.9.0 → v1.10.0](pre-commit/mirrors-mypy@v1.9.0...v1.10.0)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix bug for get dates (#284)

* fix bug for get dates

* add comments to clarify

---------

Co-authored-by: mirzaees <smirzaee@aurora.jpl.nasa.gov>

* temporary fix for compressed SLC metadata (#287)

* temporary fix for compressed slc metadata

Co-authored-by: mirzaees <smirzaee@aurora.jpl.nasa.gov>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* [pre-commit.ci] pre-commit autoupdate (#286)

updates:
- [github.com/astral-sh/ruff-pre-commit: v0.4.2 → v0.4.3](astral-sh/ruff-pre-commit@v0.4.2...v0.4.3)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Enforce convention for conncomp files to be consistent with unw files (#288)

* Do not add COR suffix, in case CONCOMP file

* Update constants for conncomp

* fix names

* add fig placeholder, new ref

fix makefile

fix paper instructions

add ref

more refs

refs

draft to end

add paper action

missing space

no need to run full test for this branch

typo

fix ref

+

refs, typos

* reword example

* This is a combination of 2 commits.

reword example

frontmatter yaml

* edits for draft

* add image for figure plot

* add SNWG ack.

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Simran S Sangha <sssangha@ucla.edu>
Co-authored-by: Ryan Burns <rtburns@jpl.nasa.gov>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Simran S Sangha <ssangha@trappist.jpl.nasa.gov>
Co-authored-by: Sara Mirzaee <37273875+mirzaees@users.noreply.github.com>
Co-authored-by: mirzaees <smirzaee@aurora.jpl.nasa.gov>
Co-authored-by: Geoffrey Gunter <12984092+gmgunter@users.noreply.github.com>
@mirzaees mirzaees deleted the interpolation_for_unwrap branch February 24, 2026 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants