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

Create prep_hyp3_crop.py #792

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

Conversation

Arctic-Ambrun
Copy link

@Arctic-Ambrun Arctic-Ambrun commented Jun 14, 2022

Provide a script for processing hyp3_product.
Including crop, transfer all images from WGS84 UTM into WGS84 lat/lon to avoid the

Co-Authored-By: Lei [email protected]

Description of proposed changes

The projection of hyp3 product is WGS84 UTM, however, It may lead a distortion when processing from vel.h5 into *.kmz

We reprojected the GTiff from hyp3, and provide a choice to crop the image into a small scale to save RAM memory and improve speed. For future update, we plan to add more choices to improve the selecting of original hyp3-gamma product.

This is my first time to pull a request, I dont know what means in Reminders

Reminders

  • Fix #xxxx
  • 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.

Provide a script for processing hyp3_product.
Including crop, transfer all images from WGS84 UTM into WGS84 lat/lon.

Co-Authored-By: Lei <[email protected]>
@welcome
Copy link

welcome bot commented Jun 14, 2022

💖 Thanks for opening this pull request! Please check out our contributing guidelines. 💖
Keep in mind that all new features should be documented. It helps to write the comments next to the code or below your functions describing all arguments, and return types before writing the code. This will help you think about your code design and usually results in better code.

@Arctic-Ambrun
Copy link
Author

WE found there exists problems in zipped files, will update a new version before this Friday.

@Arctic-Ambrun
Copy link
Author

made a script to crop hyp3 image and transfer the coordinate from UTM to latlon

@yunjunz yunjunz added this to the UTM coordinate support milestone Jul 26, 2022
@yunjunz
Copy link
Member

yunjunz commented Jul 27, 2022

Hi @Arctic-Ambrun, first, thank you for contributing! I would have hoped you opened an issue first, before implementing this PR, as described in our contributing guideline. Then the PR would be more focused. With the current form, it requires major changes as listed below:

We are determined to support UTM coordinates (as this will be used by NISAR products). So re-projecting during the data loading step is not desired. However, re-projection in the last step of save_kmz.py is desired, due to the limitation of Google Earth, as we discussed in #686. Thus, I was hoping that you may write a new function, named utm_to_latlon() for example, and add it to mintpy.utils.utils0.py. It could be something as below:

def utm_to_latlon(data, meta):
    # where data is a 2D or 3D np.ndarray, for the data in UTM coordinates
    #       meta is a dict, for the cooresponding metadata read from readfile.read_attribute()

    # code block to re-project the data from UTM to lat/lon coordinates
    # code block to update the meta dict object, reflecting the changes of the metadata related with coordinates, e.g. Y/X_FIRST/STEP/UNIT, REF_LAT/LON, the complete list is here: https://mintpy.readthedocs.io/en/latest/api/attributes/

    # then return the data in lat/lon coordinates and the updated meta dict.

I believe your hyp3_reprojection() is already very close to this. Once we modify it to the form above in utils0.py, we could call it from save_kmz.py. And this deserves its own PR.


The rest of the script seems to unzip and/or crop the HyP3 GeoTIFF files, which could be a simple replacement of the current preparation procedures using hyp3lib in cmd or in notebooks if I understood it right?

  1. The current script uses hyp3lib which has several dependencies, adding quite some new dependencies to mintpy, and we don't want that. Is it possible to implement this functionality purely based on gdal and shapely, which mintpy currently depends on? Otherwise, I think it's better to contribute this script to the hyp3lib repo, which won't have the new dependencies issue for this script.

  2. The content of the script should be merged into the existing prep_hyp3.py, instead of creating a new one. We are moving toward less number of scripts, and more sub-modules. The functionality of unzipping and/or cropping could be exposed in cmd options as:

  • prep_hyp3.py --snwe for the cropping
  • auto-determine if unzipping is required, based on the input file extension, without options in command line.

@Arctic-Ambrun
Copy link
Author

Hi @yunjunz , I understand and I will try to satisfy the requirements. But so far, the script is only works for my requirement. As a freshmen in cooperation in GITHUB, I`m really sorry. I need time for understanding the whole script and try to change untill satisfied.

  1. I tried to use utm_to_latlon function, the coordinate is always wrong. I also changed the parameters of earth radius but not work. In my opinion, use gdal_wrap is only method which successfully change the image coordinates from UTM to latlon. I suspect the reason maybe there needs more informations in the meta file. Another method for changing the UTM into correct latlon is add a reference image, then change the information in geometryGeo.h5.

  2. I will consider whether if change these scripts in the dependence library. To avoid importing too much libraries.

  3. I removed the unzip choices because its not good yet, and I`m planning to add more choices in the prepare data process. Its 40% complete but not 100% planning. My script may not satisfy the requirement above, but I will try to do it. Thanks for your reply!

@jhkennedy
Copy link
Collaborator

@yunjunz and @Arctic-Ambrun , a few thoughts the HyP3 side:

  • using hyp3lib is not recommended, and likely to change drastically, as is stated in the hyp3lib README:

    HyP3-lib

    Common library for HyP3 plugins

    Please note that the use of this library is NOT recommended.

    This library is outdated and largely untested, containing a broad collection of scripts/tools developed for upstream packages that have since undergone many major releases. HyP3 only uses a subset of this repository, and future development will include drastic changes to this repository to reduce the scope of this library. We suggest not using this library, especially in a production environment.

    If you're a MintPy user looking to crop HyP3 products, please see our Time Series Analysis with HyP3 and Mintpy Tutorial.

    For other uses, we suggest checking out our asf-tools Python library and/or the HyP3 Python SDK.

  • Most of the work in this script is around unzipping and cropping HyP3 products, however, the hyp3_sdk already provides unzip functionality, which can be used right after downloading the products:

    hyp3_products = jobs.download_files(data_dir)
    hyp3_product_folders = [hyp3_sdk.util.extract_zipped_product(product) for product in hyp3_products]

    This is shown in our MintPy tutorial here:
    https://hyp3-docs.asf.alaska.edu/tutorials/mintpy#2.-Request-On-Demand-InSAR-products-from-ASF-HyP3

    Likewise, cropping is covered here:
    https://hyp3-docs.asf.alaska.edu/tutorials/mintpy#3.1-Subset-all-GeoTIFFs-to-their-common-overlap

    The two functions shown there are fully copy-paste-able from the cells (the cells have all the needed imports).

So, I'm curious about couple things:

  1. How are you downloading HyP3 data?

  2. Would it make more sense to put preparing HyP3 products for MintPy in the hyp3_sdk? We could definitely add the two cropping functions to the SDK, and possibly even writing a MintPy config file.

    I'm not actually sure where we should seperate hyp3 maintained stuff and MintPy stuff....

  3. Do you need/want command line scripts for this? We've not added many CLI entrypoints to the HyP3 SDK, but we could.

@jhkennedy
Copy link
Collaborator

I'm not actually sure where we should separate hyp3 maintained stuff and MintPy stuff....

To follow up on this a little, the majority of usage we (HyP3) sees is SBAS InSAR stacks that are then going to be analysed in MintPy. So we're interested in supporting that workflow and making it as easy as possible for our users. But, HyP3 and MintPy are separate services/tools, both of which are broader scoped than just HyP3 -> MintPy workflows, so we should be careful to not blur the lines too much.

@yunjunz
Copy link
Member

yunjunz commented Jul 27, 2022

@jhkennedy This tutorial (https://hyp3-docs.asf.alaska.edu/tutorials/mintpy) is very nice, could you add the link to
https://mintpy.readthedocs.io/en/latest/dir_structure/#asf_hyp3, or maybe replace the current preparation content with it?

I was not aware of the lv_phi file and there is no special conversion for it in mintpy during data loading. Here is the definition in isce2 and used in mintpy. Are they the same? If so, could you please update the "mintpy.readthedocs.io" page for this as well?

Would it make more sense to put preparing HyP3 products for MintPy in the hyp3_sdk? We could definitely add the two cropping functions to the SDK, and possibly even writing a MintPy config file.

Yes, I agree, especially if these steps require hyp3* libraries.

Do you need/want command line scripts for this? We've not added many CLI entrypoints to the HyP3 SDK, but we could.

I think command line scripts would be great (especially for users who are not familiar with notebooks). It seems to me, maybe, one script for each section (as shown in the tutorial notebook). The optional custom SNWE cropping, as intended in this PR, would be helpful to be included in the hyp3 tutorial and cmd scripts.

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.

3 participants