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 passing run info from Geogrid to autoRIFT directly #18

Merged
merged 10 commits into from
Mar 11, 2021

Conversation

jhkennedy
Copy link
Contributor

Note: This PR is required by leiyangleon/Geogrid#7 and allows that PR to be leveraged in autoRIFT

This:

  • Adds some attributes to the geogrid object that are needed by autoRIFT: pOff, lOff, pCount, lCount, X_res, and Y_res. Previously, the only way autoRIFT could receive these values was by fgrep-ing a textGeogrid.txt log file, which requires subprocessing out Geogrid and makes it harder to setup GDAL + AWS requester pays environments.

  • generateAutoriftProduct and runAutorift now has a geogrid_run_info keyword argument to accept the Geogrid run_info dictionary

  • maskname = ...+"masks.tif" has been corrected to maskname = ...+"sp.tif" as listed in the JPL parameter file

  • generateAutoriftProduct now returns the name of the output netCDF file

  • The netCDF file name's percent valid pixels part has been significantly simplified. Previously, if PPP=99.6734, then that percent valid pixels would be reported in the file name like *_P996.nc. From a conversation with Mark F., this really should be a 0--100 range, where if every pixel was valid, you'd get _P100.nc (previously, you'd actually get _P1000.nc).

    Now, if PPP=996734, the percent valid pixel part of the file name will be _P096.nc and always 3 digits.

    If the previous accuracy (4 digit) is desired, you could change the code to:

    PPP = roi_valid_percentage * 1000
    0ut_nc_filename = f"..._P{np.floor(PPP):04.0f}.nc"

    Note: this will now have 4 fixed digits instead of "3 but possibly 4".

  • The grid resolution in the netCDF file name (e.g., G0240) is now variable based on the DEM resolution

  • Landsat-8 scene names are no longer truncated by 1 character in the netCDF file name

testautoRIFT.py Outdated
pixsizex = float(str.split(subprocess.getoutput('fgrep "X-direction pixel size:" testGeogrid.txt'))[-1])
else:
chipsizex0 = geogrid_run_info['chipsizex0']
pixsizex = geogrid_run_info.get('pixsizex', geogrid_run_info['XPixelSize'])
Copy link
Collaborator

Choose a reason for hiding this comment

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

this could be simplified as pixsizex = geogrid_run_info['XPixelSize'] as pixsizex should not be declared before here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That matches the previous logic -- set pixsizex to the ground range pixel size for radar, as reported here:
https://github.com/leiyangleon/autoRIFT/blob/master/geo_autoRIFT/geogrid/src/geogrid.cpp#L657
otherwise fall back to the X-direction pixel size.

This info should/will be set in the run-info block if the FIXMEs in leiyangleon/Geogrid#7 are addressed. Specifically, this one:
https://github.com/jhkennedy/Geogrid/blob/remote-access/testGeogrid_ISCE.py#L302

Are you saying this should always be the X-direction pixel size?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, let us make the pixelsizex always referring to XPixelSize in georgid runinfo, for simplicity. The reasons are two fold.

  1. you have other variables in georgid runinfo being the same for radar and optical, and they are also named by the "x/y" convention. See "xoff" and "xcount" of runinfo for both radar and optical in your geogrid PR.
  2. This is okay since range is usually considered x and azimuth is y in radar imagery.

Once the FIXME's are addressed, I will put something like 'XPixelSize': obj.grd_res for radar vs. 'XPixelSize': obj.X_res for optical as you did for the georgid runinfo. In other words, the naming convention only affects testGeogrid not geo_autoRIFT, and is set as such to simplify your workflow with geogrid runinfo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good! Done.

@leiyangleon
Copy link
Collaborator

Please fix my minor comments. LGTM otherwise. Will merge it then.

@leiyangleon leiyangleon merged commit ef6be08 into nasa-jpl:master Mar 11, 2021
@jhkennedy jhkennedy deleted the remote-access-2 branch March 19, 2021 16:45
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.

2 participants