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

add aerosol related updates #161

Merged
merged 13 commits into from
Jan 14, 2025

Conversation

bbakernoaa
Copy link
Contributor

This closes #160

Main updates were in the template.py and _grib2io.py files. More improvement could be completed later but it does produce a grib2 message now with unique shortNames and fullNames for each variable now.

image

Additionally the xarray backend works as well.

image

image

@bbakernoaa
Copy link
Contributor Author

@zmoon

@bbakernoaa
Copy link
Contributor Author

This is still in a draft mode. Please don't merge yet

@bbakernoaa
Copy link
Contributor Author

This should be ready now.

@bbakernoaa
Copy link
Contributor Author

ok should be good to go. More work is needed but this at least adds the capability for GEFS-Aerosols and RRFS to properly read the variables and assign unique variable names to each aerosol species.

Copy link
Contributor

@zmoon zmoon left a comment

Choose a reason for hiding this comment

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

Just some cleanup comments. Also maybe the aerosol-specific get short/full name sections could be extracted to their own function/method.

log Outdated Show resolved Hide resolved
src/grib2io/_grib2io.py Outdated Show resolved Hide resolved
src/grib2io/xarray_backend.py Outdated Show resolved Hide resolved
src/grib2io/templates.py Outdated Show resolved Hide resolved
src/grib2io/templates.py Show resolved Hide resolved
src/grib2io/templates.py Outdated Show resolved Hide resolved
src/grib2io/xarray_backend.py Outdated Show resolved Hide resolved
src/grib2io/xarray_backend.py Outdated Show resolved Hide resolved
src/grib2io/xarray_backend.py Outdated Show resolved Hide resolved
@EricEngle-NOAA
Copy link
Collaborator

How is this coming along? Do you think it is ready to merge?

@bbakernoaa
Copy link
Contributor Author

Yes I believe it is ready to go. This actually led us to find an error in the way they were assigning a grib variable in the RRFS prototype too. They were assigning the aerosol type as "Missing" for some of their species instead of total or 62000

src/grib2io/tables/section4.py Show resolved Hide resolved
src/grib2io/_grib2io.py Show resolved Hide resolved
src/grib2io/templates.py Show resolved Hide resolved
log Outdated Show resolved Hide resolved
@EricEngle-NOAA
Copy link
Collaborator

Forgot to post these questions from my review a few days ago. Apologies.

@bbakernoaa
Copy link
Contributor Author

@EricEngle-NOAA Anything you want me to do to get this merged?

@EricEngle-NOAA
Copy link
Collaborator

@bbakernoaa can you create a test for this? See the tests/ dir at the repo root. We are using pytest. Also feel free to add a GEFS chem file to tests/data/, but thin the grid to 1.0 deg to keep a smaller file size.

@EricEngle-NOAA
Copy link
Collaborator

I think a test to make sure the shortName and fullName are properly constructed is good enough. That is all that this PR is for so no need to test data unpacking etc.

@bbakernoaa
Copy link
Contributor Author

Ok will do. I added the 2d data file here but may also add one for the 3d data

@EricEngle-NOAA
Copy link
Collaborator

Any updates regarding a test?

@bbakernoaa
Copy link
Contributor Author

bbakernoaa commented Dec 9, 2024 via email

@EricEngle-NOAA
Copy link
Collaborator

@bbakernoaa - No problem. Thanks for the update.

@EricEngle-NOAA
Copy link
Collaborator

Hi @bbakernoaa - Any update on this?

@bbakernoaa
Copy link
Contributor Author

@EricEngle-NOAA please take a look and let me know if that test works

@bbakernoaa
Copy link
Contributor Author

@EricEngle-NOAA Sorry removed an unused import that snuck in.

@EricEngle-NOAA
Copy link
Collaborator

Thanks @bbakernoaa and sorry for being a pain. I might do some moving around of what you added.

@bbakernoaa
Copy link
Contributor Author

please do!

@EricEngle-NOAA EricEngle-NOAA merged commit 4b19aee into NOAA-MDL:master Jan 14, 2025
10 checks passed
@bbakernoaa
Copy link
Contributor Author

@EricEngle-NOAA I noticed the gefs grib file is in the base directory along with in the tests/data directory. We should probably remove the one in the base directory

@EricEngle-NOAA
Copy link
Collaborator

I did move it under tests/data/ in the commit I am working on.

@bbakernoaa
Copy link
Contributor Author

bbakernoaa commented Jan 22, 2025 via email

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.

Not able to read the GEFS-Aerosol grib2 data
3 participants