Update inline post #666
Update inline post #666
Conversation
There was a problem hiding this comment.
@WenMeng-NOAA did you make changes to add 160m and 320m wind components in this PR as in my PR #693? I'm not seeing those changes here.
I think it sounds like a good idea to sync the updates of the sndepac calculation back to the offline post. If you can add it to your PR, that would be great.
@EricJames-NOAA This PR is for updating inline post interface with the changes from offline post interface. The changes for 160 and 320m wind components will be included in inline post with submodule upp revision upgrade in this PR. |
|
Are there regression tests in ufs-weather-model that run with gocart_on, gccpp_on or nasa_on? |
|
I see huge code duplication between post_fv3.f90 and INITPOST_NETCDF.f in UPP, for example code that computes various PM concentration. All computations should be in done in the post library not here or in INITPOST. The only purpose of this module (post_fv3) is to pass model data to post library not to perform diagnostic computations. Similarly the purpose of INITPOST_NETCDF should be just to read model data from external file not to perform diagnostic computations. Maintaining very similar or exactly the same code (non trivial code) at to places is very error prone and in my opinion unnecessary. |
I am not sure. From my testing, I didn't see gocart_on, gccpp_on and nasa_on. @zhanglikate @junwang-noaa Do you know that? |
@DusanJovic-NOAA Thanks for the good suggestion. From UPP code management perspective, I totally agree with you. I am looking into the UPP code, this kind of enhancement might be in the interfaces of offline post for nemsio (GEFS V12) and netcdf (GFS17/GEFS12) and the interface of inline post. Given the resource constraints from UPP, I would suggest the effort starts after nemsio interface removal from UPP. Please let me know your comments on my proposal. @zhanglikate may chime in since she is the main developer for aerosol/chemical fields in UPP. |
There is no regression test for gocart_on option now since this is the GEFS-Aerosols model. I am still working on create the regression test for gccpp_on since this part has not been merge to current UFS model. For nasa_on, which is for current UFS-Aerosols model, not sure @junwang-noaa or @lipan-NOAA may have tested it. Thanks. |
|
@DusanJovic-NOAA @junwang-noaa Should we let this pr move forward while additional regression tests for these gocart_on/gccpp_on/nasa_on may follow on in future? @WenMeng-NOAA I am wondering if we need to create issues pointed in this conversation. So we can track as PRs move. |
|
We can merge this PR as it is now, and add regression tests and refactor later. |
I will create an issue regarding comments from @DusanJovic-NOAA and @junwang-noaa for inline post enhancement. |
|
Testing is complete on #1794. Once we have one more approval here, the merge process can begin. |
|
Ok, we have two approving reviews. @BrianCurtis-NOAA please feel free to merge this PR. |
|
Double checking hashes, upp isn't at the top of their develop and I was wondering if it's purposely lagged? @WenMeng-NOAA or @DusanJovic-NOAA know? |
@BrianCurtis-NOAA Yes, the recent commits from UPP/develop haven't being tested in in-line post yet. |
|
@WenMeng-NOAA Thank you! @jkbk2004 why was this merged before my question was answered? |
|
@BrianCurtis-NOAA upp develop hash continues to march, right? @WenMeng-NOAA https://github.com/NOAA-EMC/UPP/tree/baa77515f182eb6d6f2f8b57bd1e0ffb742761e8 hash is correct, right? Let me know if we need to revert. |
|
@jkbk2004 and @BrianCurtis-NOAA The upp @baa7751 is the correct revision for this PR. |
|
@WenMeng-NOAA So just to be clear, it appears that UPP is the one repo that is not carried in UFS at the top of the develop branch. I admit to not paying a lot of attention to the UPP updates---but why is UPP a special case? All other components point to the top of the develop branch of their respective repos. |
|
@DeniseWorthen When I submitted the FV3 PR, the upp revision is the top of develop branch. But this PR has been waiting over one month though I updated upp revisions several times. Meanwhile The UPP develop has been updated frequently. I don't want the upp commits from this week break UFS without inline post testing. |
|
@WenMeng-NOAA Thanks. But for all other components (eg. CICE, MOM6, CMEPS), we cannot update the develop branch for a component w/o simultaneously updating the UFS. They always go hand-in-hand. But it appears that for UPP, you can update develop at any time. |
|
@DeniseWorthen The UFS users can configure the UPP as a standalone utility (offline post) or sub component of FV3 (inline post) to post process model output. Some updates in UPP/develop are for offline post only and some updates will be applied in inline post with changes in the interface at FV3. I usually upgrade upp revision at FV3 every two months. |
|
@WenMeng-NOAA Thanks for the explanation. |
Description
(Instructions: this, and all subsequent sections of text should be removed and filled in as appropriate.)
This PR include changes are:
Issue(s) addressed
Link the issues to be closed with this PR, whether in this repository, or in another repository.
(Remember, issues should always be created before starting work on a PR branch!)
Testing
How were these changes tested?
What compilers / HPCs was it tested with?
Are the changes covered by regression tests? (If not, why? Do new tests need to be added?)
Have the ufs-weather-model regression test been run? On what platform?
Dependencies
If testing this branch requires non-default branches in other repositories, list them.
No.
Do PRs in upstream repositories need to be merged first?
No.