-
Notifications
You must be signed in to change notification settings - Fork 19
ULTRA l2 map attributes #2175
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
base: dev
Are you sure you want to change the base?
ULTRA l2 map attributes #2175
Conversation
@@ -196,6 +196,50 @@ exposure_factor: &exposure_factor | |||
DISPLAY_TYPE: no_plot | |||
DICT_KEY: SPASE>TemporalDescription:Exposure,Qualifier:Directional,CoordinateSystemName:HAE,CoordinateRepresentation:Spherical | |||
|
|||
geometric_function: |
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.
I think this should be geometric factor?
geometric_function: | |
geometric_factor: |
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.
I had the same thought but apparently they want geometric function. Will that be going against the grain? Maybe it is worth it to have a separate ultra l2 map yaml file.
DISPLAY_TYPE: no_plot | ||
DICT_KEY: SPASE>Particle>ParticleType:Atom,ParticleQuantity:GeometricFactor,Qualifier:Directional,CoordinateSystemName:HAE,CoordinateRepresentation:Spherical | ||
|
||
efficiency: |
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.
Some of these are ultra-specific and will be different for Lo/Hi I believe. Do we want to add a separate file like enamaps_l2-ultra
and similar for some of these?
On Lo, our efficiency is per pointing (so not on the map), and it has a dependence on energy. I'm not sure if yours does or not (looks like no 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.
I see, the energy depend was defined in the specific healpix/latlon files. I guess that could be brought out here since it is the same for all of them, but it doesn't really matter. I guess you can ignore this comment for now.
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.
Oh interesting. So the efficiency just has energy as a depend for lo?
# Rename positional uncertainty variables if present | ||
if "scatter_theta" in map_dataset and "scatter_phi" in map_dataset: | ||
map_dataset = map_dataset.rename( | ||
{"scatter_theta": "positional_uncertainty_theta"} | ||
) | ||
map_dataset = map_dataset.rename({"scatter_phi": "positional_uncertainty_phi"}) |
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 won't fail if the variables aren't present, so you can just call the rename directly and it will work if they are there.
# Rename positional uncertainty variables if present | |
if "scatter_theta" in map_dataset and "scatter_phi" in map_dataset: | |
map_dataset = map_dataset.rename( | |
{"scatter_theta": "positional_uncertainty_theta"} | |
) | |
map_dataset = map_dataset.rename({"scatter_phi": "positional_uncertainty_phi"}) | |
# Rename positional uncertainty variables if present | |
map_dataset = map_dataset.rename({"scatter_theta": "positional_uncertainty_theta", "scatter_phi": "positional_uncertainty_phi"}) |
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.
My only comment is that Ultra IT is adding some variables to the L2 definition a bit ad hoc. I guess in the name of progress, that is OK with me.
@@ -220,6 +264,18 @@ obs_date_range: | |||
DISPLAY_TYPE: image | |||
TIME_SCALE: Terrestrial Time | |||
|
|||
obs_date_for_std: |
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.
The product definition agreed to by the IMWG named this obs_date_range
. Is Ultra requesting a change in that variable name? Here is the spreadsheet with the current definition: https://o365coloradoedu-my.sharepoint.com/:x:/g/personal/plummert_colorado_edu/EX8IRmgRKw5Hpik9wPAXZQABBgOemDqOv-uLMlRxZLJaTw
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.
Oh, I guess I am confused what this is vs. obs_date
and obs_date_range
.
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.
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.
Ok. I'm not sure what to do for this. Probably move forward with what you have. I will send out an email to the IMWG noting that I am seeing lots of variables being added to L2 products last minute here. All the ENA ITs agreed to the spreadsheet and this is causing quite a bit of churn.
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.
OK I also just messaged Nick Dutton for more info. Yeah sorry about that - I didnt realize these are unexpected.
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.
At least for this issue im wondering if they will be ok with just obs_date and obs_date range, knowing now that obs_date_range seems to be the std already.
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.
Just one question about the uncertainty name. Everything looks good after you respond to Greg and Tim.
LABL_PTR_2: longitude_label | ||
LABL_PTR_3: latitude_label | ||
|
||
positional_uncertainty_theta: |
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.
Based on the recent Slack messages should this now be uncert instead?
Change Summary
Overview
Update ena map attributes to include efficiency, geometric function, and positional uncertainty.
Updated Files
Testing