Enable gcafsv1 aerosol GRIB2 processing in UPP#4147
Enable gcafsv1 aerosol GRIB2 processing in UPP#4147aerorahul merged 10 commits intoNOAA-EMC:developfrom
Conversation
zhanglikate
requested review from
DavidHuber-NOAA,
TerrenceMcGuinness-NOAA,
WenMeng-NOAA and
aerorahul
as code owners
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
aerorahul
left a comment
aerorahul
left a comment
There was a problem hiding this comment.
Thanks for the PR.
Some comments
|
@zhanglikate |
|
Adding @JessicaMeixner-NOAA as a reviewer since this PR contains a |
JessicaMeixner-NOAA
left a comment
JessicaMeixner-NOAA
left a comment
There was a problem hiding this comment.
I was asked to review because of the update of hash in ufs-weather-model. I do not think that this PR will work without further updates that are associated with various ufs-weather-model updates. Have the CI tests been run with this? My guess is that they will fail.
I plan to work on a PR with updates from UFS hashes. I was going to wait until the PR that is being tested today in ufs was merged because that's what we need for GFSv17. I can put together a branch with those updates this afternoon without that, to help expedite this PR if waiting until Monday is not okay.
The related update about this PR in WM has been updated and merged: ufs-community/ufs-weather-model#2957 (comment) |
|
The testing section of your PR is empty, how have you tested your PR here? Is it working without additional modifications to the workflow? |
For the WM part (ufs_model.fd), it has passed all the RT. |
scripts/exglobal_atmos_products.sh
Outdated
| # if at final record and have not reached the final processor then write echo's to | ||
| # cmdfile for remaining processors | ||
| if [[ ${last} -eq ${ncount} ]]; then | ||
| <<<<<<< HEAD |
Check failure
Code scanning / ShellCheck
Couldn't parse this here string. Fix to allow more checks.
@aerorahul I have merged it with develop branch, thanks. |
|
Updates required to make the model run can be found here: #4210 I plan to wait until the current PR in UFS that's being tested is merged before opening the PR. That should be Monday, but its here for reference in the meantime. |
@zhanglikate - Do you mean the ufs-weather-model RT or the global-worklfow CI tests? Passing the ufs-weather-model RT's is not sufficient to ensure the global-workflow will run the model. That being said, #4210 has been merged. If you merge that in your PR here this should be fine. |
@JessicaMeixner-NOAA please check ufs-community/ufs-weather-model#2957 (review) and ufs-community/ufs-weather-model#2942 , I was following the guidance to finish the RT run and push to the PR in WM. |
JessicaMeixner-NOAA
left a comment
JessicaMeixner-NOAA
left a comment
There was a problem hiding this comment.
@zhanglikate - Can you update the hash that ufs-weather-model is pointing to here? I think you should be using what is currently in the develop of global-workflow and not another hash.
@JessicaMeixner-NOAA I was originally using the global workflow hash for ufs-weather-model, when I submit the PR to ufs-weather-model, the code manager asked me to use the most recent hash for ufs-weather-model and run the RT #4210. Actually, I only added one file into the ufs-weather-model which will not impact on other model's configurations, however, it links to all the updates in these global workflow PR. Please guide me to what is the best way to proceed this global workflow PR. Thanks very much. |
|
Right now, you are not using the same ufs-waether-model hash as what was used in #4210. Seems like things got unintentionally out of sync. You just need to update your hash of ufs-weather-model to what is being used in global-workflow develop right now and you'll be good to go. You can confirm you've done that when you look at the files changed in this PR and you do not see ufs_model.fd as being changed. |
@JessicaMeixner-NOAA I revert the ufs-weather-model to what is being used in global-workflow develop, can you please help to check? Thanks. |
|
@zhanglikate - The ufs-weather-model hash is now as expected. |
zhanglikate
force-pushed
the
PR-upp
branch
3 times, most recently
from
b25b189 to
159c61f
Compare
|
@aerorahul HI Rahul, I have pushed a version to solve the conflicts (159c61f), that what it always shows "This branch has conflicts that must be resolved"? Thanks. |
|
@zhanglikate, if you click on resolve conflicts, make updates and push them, the conflicts will disappear. |
|
C96C48mx500_S2SW_cyc_gfs FAILED on Hercules (pipeline ID: 6239) In directory: Error Log Files: View Error Logs: (gfs_wavepostbndpnt.log) This failure was detected automatically by global-workflow's CI/CD Pipeline |
@aerorahul Can you please let me know how to turn off "paramlistb" in the PR testing and only run "paramlista" as the current GCAFS configurations need to turn off "paramlistb" and the master file does not include the variables for "paramlistb", which will be updated in future PR. |
emcbot
added
the
CI-Ursa-Building
TerrenceMcGuinness-NOAA
removed
the
CI-Ursa-Failed
|
@zhanglikate that is unreasonable to ask from code managers such as @aerorahul , it is the responsibility of developers to ensure that all necessary changes are included in the pull request. It's not even clear to me exactly would need to be changed to fix this, so someone who isn't familiar with GCAFS probably wouldn't know either. Have you ran the GCAFS test cases on your own manually using |
@CoryMartin-NOAA Thank you for the comments. To clarify, this is not a bug in the code that needs to be fixed. The UPP production workflow currently uses two parameter lists, paramlista and paramlistb. In the develop branch of GCAFS, the GCAFS control file for grib2 variables has not been finalized. So the current master GRIB2 output contains only aerosol species and does not include any meteorological fields. All aerosol species are already included in paramlista, and at the moment we are not using paramlistb. This is why the PR test fails—UPP attempts to process an aerosol-only master file using paramlistb, which currently contains only meteorological variables. I could temporarily update paramlistb by adding one aerosol field to avoid the crash. However, my understanding is that @lipan-NOAA plans to use paramlistb soon for meteorological or pressure-level outputs and update the GCAFS control file for grib2 variables. Because of this planned change, I checked with the UPP code manager @WenMeng-NOAA before modifying anything. She suggested asking @aerorahul whether the PR test could temporarily use an option/configuration to disable paramlistb processing for GCAFS model, so the test can complete successfully now. Once the meteorological fields are finalized, we would re-enable paramlistb in the production workflow. Please let me know whether this clarification helps or if you would prefer a different approach for this PR. If @aerorahul confirms that it is not feasible to use an option or configuration to disable paramlistb processing for the PR test, I can update the gcafs.fFFF.paramlist.b.txt file by adding one aerosol field so that the test can pass. Thank you. |
|
I think setting |
@aerorahul Thanks very much for the information. I will update parm/config/gcafs/config.atmos_products to temporally turn off the paramlistb until @lipan-NOAA finalize the paramlistb. |
zhanglikate
dismissed stale reviews from bbakernoaa and aerorahul
via
346e4bf
TerrenceMcGuinness-NOAA
removed
the
CI-Ursa-Building
aerorahul
removed
CI-Hercules-Failed
emcbot
added
CI-Hercules-Ready
aerorahul
left a comment
aerorahul
left a comment
There was a problem hiding this comment.
test passed on Hercules. approving before merging.
@aerorahul I really appreciate all your help and patience with this PR. |
# Description This PR resolves issue ##4233 This PR adds the capability to process analysis tracer fields into aerosol AOD product in GRIB2 format using the upp tool within the GCAFS workflow. The input for UPP is aerosol analysis on Gaussian grid. Aerosol variables in atmospheric analysis file (`analysis.atm.a006.nc`) are all missing data, such that the aerosol Gaussian tracer analysis fields are generated by adding aerosol increments (aeroinc_gaussian) to the `atm.f006` background file. This feature is implemented in `analcalc`, `atmanlupp`, and `atmanlprod` jobs in GCAFS workflow. This PR depends on parm/product/gcafs* files, such that it will work after #4147 merged. Resolves #4233 # Dependency PR #4147 # Type of change - [ ] Bug fix (fixes something broken) - [ ] New feature (adds functionality) - [ ] Maintenance (code refactor, clean-up, new CI test, etc.) # Change characteristics <!-- Choose YES or NO from each of the following and delete the other --> - Is this a breaking change (a change in existing functionality)? NO - Does this change require a documentation update? NO - Does this change require an update to any of the following submodules? NO (If YES, please add a link to any PRs that are pending.) # How has this been tested? - CI test C96_gcafs_cycled, C96_gcafs_cycled_noDA on Hera # Checklist - [ ] Any dependent changes have been merged and published - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have documented my code, including function, input, and output descriptions - [ ] My changes generate no new warnings - [ ] New and existing tests pass with my changes - [ ] This change is covered by an existing CI test or a new one has been added - [ ] Any new scripts have been added to the .github/CODEOWNERS file with owners - [ ] I have made corresponding changes to the system documentation if necessary --------- Co-authored-by: Yaping Wang <[email protected]> Co-authored-by: ypwang19 <[email protected]> Co-authored-by: Cory Martin <[email protected]> Co-authored-by: Barry Baker <[email protected]> Co-authored-by: Travis Elless <[email protected]>
8 participants
Description
This PR updates the UPP system to support aerosol GRIB2 output for the gcafsv1 model within UFSATM.
Specifically:
Updates UPP to the develop branch to include the text file required for the GCAFS model in UFSATM.
Adds the post_itag_gcafs file for the GCAFS model.
Implements and links UPP-related components for the gcafsv1 model to process aerosol GRIB2 outputs.
These changes enhance UFSATM’s capability to handle aerosol fields generated by GCAFS for post-processing and analysis.
Type of change
Change characteristics
How has this been tested?
Checklist