-
Notifications
You must be signed in to change notification settings - Fork 2
DEM Logic and Backscatter Convention Specification #84
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
Conversation
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 looks good, thanks Alex! I've added a few comments/questions/suggestions for you to have a look at
| "_mask.tif", | ||
| ".png", | ||
| ], | ||
| "sigma0": [ |
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.
Do we want to do anything at this time to signify whether we're producing the dem- or ellipsoid-version of sigma0? (sorry, this one still confuses me, so I'm not even 100% clear on which one is produced by default)
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.
Decision to leave as is and make the change later
| "_HV_gamma0.tif": "HV polarised gamma0 linear backscatter", | ||
| "_VV_gamma0.tif": "VV polarised gamma0 linear backscatter", | ||
| "_VH_gamma0.tif": "VH polarised gamma0 linear backscatter", | ||
| "_HH_sigma0.tif": "HH polarised sigma0 linear backscatter", |
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.
Should the description say whether it's dem vs ellipsoid sigma0?
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.
Yeah I think I will add to the description here and add a logger.warning statement in the workfow to let the user know the sigma0 data is dem referenced, and ellipsoid referenced should be made using the incidence angle. Unfortunately it's not very clean... what do you think?
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.
Decision to leave as is and make the change later
| ".png": pystac.media_type.MediaType.PNG, | ||
| } | ||
|
|
||
| UPDATED_METADATA_PARAMETERS = { |
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.
Confirming that this is no longer required because we're using the GA-fork version of the code?
| backscatter_convention=gamma0 # gamma0, sigma0 or beta0 | ||
| s3_bucket="deant-data-public-dev" | ||
| s3_project_folder="ga_s1" | ||
| collection="s1_rtc_c1" |
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.
Does this now need to be changed to "collection_number"? Or have you not made that change just yet?
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.
collection_number logic will come in the next PR, just didn't want to make too many changes at once for y'all to review
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.
Looks good, thanks Alex!
My only feedback is that it might be worth adding a statement to the documentation of how one should generate ellipsoid-referenced backscatter_convention flag.
Co-authored-by: Caitlin Adams <[email protected]>
Co-authored-by: Caitlin Adams <[email protected]>
dem_typeto be a single value or list of preferences. The code will then iterate through the preferences and if the preferred option has dem data it will be used. For example, preferencing the REMA_32 DEM for the Antarctic regionbackscatter_coventionat the top level. This enablesgamma0,sigma0andbeta0data to be created from the top