-
Notifications
You must be signed in to change notification settings - Fork 3
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
Final GUNW 3.0 Packaging Updates #161
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
@mgovorcin - the water mask is obtained through ESA world cover 2021. I wrote a package (similar to dem-stitcher) that merges the tiles. The permanent water class is what I am using. Here is a basic example in the demo: https://github.com/OPERA-Cal-Val/tile-mate/blob/dev/notebooks/Basic_Demo.ipynb And here is how it is used in the code: https://github.com/ACCESS-Cloud-Based-InSAR/DockerizedTopsApp/blob/final_packaging_updates/isce2_topsapp/localize_mask.py#L27-L40 |
@mgovorcin Please review impact in the ionospheric area. @cmarshak we should document the limitations of our ancillaries somewhere. i.e. DEM, water mask (max latitude bounds etc). Did we also confirm there are no gaps on the water masks etc. We had some issues with the DEM and patched it earlier too,. |
I all this looks fine to me please do some testing to confirm this is all as expected. |
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.
LGTM
@mgovorcin Please open these global variables and ensure this is meeting your expectations. |
@dbekaert asked for the following:
For larger scale studies, let's use |
Also, the
|
Regarding DEM and water mask availability, if the auxiliary datasets required by the plugin are not available over a particular area, each separate package raises an exception: For the water mask: https://github.com/OPERA-Cal-Val/tile-mate/blob/dev/src/tile_mate/exceptions.py#L5 Each plugin will get as much of the tiles that are available over a requested area and raises warnings if the requested bounds are outside of the tiles that are available. I think this is a good approach. |
@dbekaert : One minor component, take it or leave it, think might be more instructive for common user to use standard convention for azimuth angle(which would be angle from North) rather than ISCE2 (from east), e.g. this example has azimuth -99deg w.r.t East that is 351deg w.r.t North (ascending track).
@cmarshak changes looks great to me, thanks |
@mgovorcin - I was told as long as we are consistent (even if consistently wrong) that's all that matters. That's what this test illustrates: Specifically, the mean values captured as global variables have the same means approximately as obtained from the rasters/cubes in the GUNW itself. |
This represents the final packaging updates requested by @dbekaert and ACCESS team. Note: the python files I modified were reformatted by
ruff
(via VS code) and so there appears to be more changes than what is done. Below, I have highlight important changes and spell-out important questions that can be answered before merging.Here is a sample GUNW run by this branch: link. The command to generate the above product is:
Here is the browse imagery:
Note: ionosphere and SET corrections are now default so the correction arguments are not needed in the above command.
Ionosphere
@mgovorcin @dbekaert:
Please review this modification to the staging of the water mask for the ionosphere here. Below are screenshots of the ionosphere layers (ionosphereBurstRamps and ionosphere, respectively) in the product (which can also be seen directly in the GUNW linked above):
I can provide additional ISCE files or you can check the folder on our shared server leffe in
/mnt/leffe-data2/cmarshak/Jan2024-topsapp-revision
.Global attributes for relevant mean values
@sssangha @mgovorcin @dbekaert
In an email from David last week, we agree to have global attributes of the following:
They are now as global attributes (copied from the
panoply
netcdf viewer):All the values except for the baselines can be obtained by averaging a band from the ISCE2 raster. The relevant paths (in the merged folder) and the band are captured here. For taking the mean of geocoded ISCE2 rasters, the code is here. I created a simple integration test by staging the
merged
directory in s3 and using those rasters to generate the means provided. If there are corrections or changes or concerns, please make specific use of this staged data so we can all be on the same page.For extracting baseline data, you can see this code here that extracts the baselines from the relevant
topsProc.xml
file and computes an average for perpendicular and parallel baseline values (there are 6 total for each value). An integration test of the extraction process is here in which the baselines are extracted from this sampletopsProc.xml
.Water Mask
The water mask used in all the workflows is the ESA Land Cover 2021 map (via tile-mate).
For the ionosphere, as noted above, the code is here. For the browse imagery, it is captured here. For the mean value of the masked coherence, it reuses the browse masking and the relevant lines of code are found here.
To see some improvement of the browse imagery from this PR compare it to the browse image generated the current
dev
branch: