Skip to content
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

Allow GPS class to work with other image projections #1079

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

scottstanie
Copy link
Contributor

@scottstanie scottstanie commented Aug 27, 2023

Description of proposed changes

  • Added two general purpose functions useful for interacting with projections other than EPSG:4326:
    • reproject convert X/Y coordinates between different EPSGs. It's more general than some of the other UTM conversion functions- for instance, to_latlon can now just call the reproject function to do the conversion.
    • get_image_rowcol to find the pixel location nearest to some lat/lon location
  • Both functions are used in the GPS class to convert the gps station locations in lat/lon to whatever the image projection is, stored in the EPSG attribute
    • Future note- I've also used this in a change to tropo_pyaps.py to run that script on non-EPSG:4326 images. That will be submitted after

Reminders

  • Partially addresses insar_vs_GPS comparison with geometry in geographical coordinates #1010
  • Pass Pre-commit check (green)
  • Pass Codacy code review (green)
  • Pass Circle CI test (green)
  • Make sure that your code follows our style. Use the other functions/files as a basis.
  • If modifying functionality, describe changes to function behavior and arguments in a comment below the function declaration.
  • If adding new functionality, add a detailed description to the documentation and/or an example.

@scottstanie scottstanie changed the title WIP: changes to Gps class to compare to other projections WIP: changes to GPS class to compare to other projections Aug 27, 2023
@scottstanie scottstanie changed the title WIP: changes to GPS class to compare to other projections Allow GPS class to compare to work with other image projections Aug 29, 2023
@scottstanie scottstanie changed the title Allow GPS class to compare to work with other image projections Allow GPS class to work with other image projections Aug 29, 2023
@yunjunz yunjunz self-requested a review August 29, 2023 07:00
Copy link
Member

@yunjunz yunjunz left a comment

Choose a reason for hiding this comment

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

Thank you @scottstanie for the PR! My main comment is the get_image_rowcol(): since it relies on mintpy.objects.coord.coordinate.geo2radar(), and mintpy.objects.coord.py relies on mintpy.utils.utils0.py, I would suggest moving this function into mintpy.objects.coord.py to avoid the circular imports. You could find the complete reliance info among sub-modules in mintpy here. The possible location could be:

  • as an independent function as it's now OR
  • inside geo2radar() OR
  • inside lalo2yx()

UPDATE: If we assume the InSAR file is already geocoded (in EPSG:4236 or other projections), then merging geo_iamge_rowcol() into lalo2yx() sounds the simplest to me. What do you think?

Could you also rebase this PR against the latest main branch?

Comment on lines +24 to +25
utils0 as ut0,
utils1 as ut,
Copy link
Member

@yunjunz yunjunz Aug 30, 2023

Choose a reason for hiding this comment

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

utils0 is imported within util1 already, so we don't need an extra utils0 as ut0 here. This is a messy part in mintpy, and the current logic is: 1) utils1 imports everything from utils0; 2) utils imports everything from utils0 and utils1.


def get_image_values(self, filename, datasetName=None, pad: int = 0, print_msg=False):
Copy link
Member

@yunjunz yunjunz Aug 30, 2023

Choose a reason for hiding this comment

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

This function is not used currently anywhere, so it's fine with me. For your information: from my experience, given a GNSS site, we either interpolate InSAR or average multiple InSAR pixels around the GNSS site. Personally, I have not gotten a good result yet by comparing GNSS and its nearest InSAR pixel yet.


return inc_angle, az_angle
return inc_angle or np.nan, az_angle or np.nan
Copy link
Member

Choose a reason for hiding this comment

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

I am just curious: in this syntax, when will the np.nan value be returned? Is it the same scenario as the raise ValueError above?

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.

None yet

2 participants