-
Notifications
You must be signed in to change notification settings - Fork 53
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
Conversation
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']) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 FIXME
s 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?
There was a problem hiding this comment.
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.
- 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.
- 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good! Done.
Please fix my minor comments. LGTM otherwise. Will merge it then. |
…PixelSize as YPixelSize for radar
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
, andY_res
. Previously, the only way autoRIFT could receive these values was byfgrep
-ing atextGeogrid.txt
log file, which requires subprocessing out Geogrid and makes it harder to setup GDAL + AWS requester pays environments.generateAutoriftProduct
andrunAutorift
now has ageogrid_run_info
keyword argument to accept the Geogridrun_info
dictionarymaskname = ...+"masks.tif"
has been corrected tomaskname = ...+"sp.tif"
as listed in the JPL parameter filegenerateAutoriftProduct
now returns the name of the output netCDF fileThe 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:
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 resolutionLandsat-8 scene names are no longer truncated by 1 character in the netCDF file name