-
Notifications
You must be signed in to change notification settings - Fork 7
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
Changes in FV3/ccpp-physics for prognostic cloud closure scheme. #66
Conversation
@dustinswales I'm not really following the changes with the new fork and the two parallel UFS/NCAR repositories. My submodule pointers are currently pointing to my fork of ufs-weather-model, fv3atm and ccpp-physics. Those were forked from ufs-community/ufs-weather-model, NOAA-EMC/fv3atm, and NCAR/ccpp-physics. My remotes are set to: [Lisa.Bengtsson@hfe09 ufs-weather-model-PR]$ git remote -v [Lisa.Bengtsson@hfe09 FV3]$ git remote -v [Lisa.Bengtsson@hfe09 physics]$ git remote -v Do I need to change my upstreams now when things are moved to NCAR? When I merge with the upstream code I usually do:
Thanks for your help! |
@lisa-bengtsson In FV3, git remote add upstream_ NCAR https://github.com/NCAR/fv3atm When the ccpp forks are "synced" next week, you can go back to just updating (fetch and pull) from "upstream". @grantfirl Did I omit anything? |
No, I think that should be sufficient. @lisa-bengtsson, if you do as @dustinswales suggested, that will give you ultimate flexibility to update from either the ufs-community/ufs/dev branch or NCAR/main. Since your work is highly geared toward the UFS, I would think that all future PRs should be targeted toward ufs/dev to avoid the (newly-added) processes of going into NCAR/main first, then ufs/dev like we're doing for this current PR. Unless there is something specific in NCAR/main that you need (that is not also in ufs/dev), future code change branches that you create should begin from the latest ufs/dev branch. |
yeah, I agree that this should be in ufs/dev, the PR was committed just before the new fork was included. I will wait until I hear more from you regarding merge into NCAR/ccpp-physics. Thanks |
@lisa-bengtsson I'm running the RT's today. If all the tests pass, with the exception of control_c384_progsigma, I will merge your PR. Then I will need you to update your submodule in fv3atm and we'll be done. |
Not your fault Swales! Poor timing on my end... it should be doable. Thanks! |
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.
Need to revert .gitmodules, but otherwise OK.
…note used by NCAR fork. Modified rt.sh for use with NCAR fork.
…or the P8 physics suite) (ufs-community#1451) * This is equivalent to PR66 in NCAR/ufs-weather-model: Bugfix and optimization of prognostic closure for the P8 physics suite Co-authored-by: JONG KIM <[email protected]> Co-authored-by: Brian Curtis <[email protected]>
Opened by @dustinswales on behalf of @lisa-bengtsson
This PR corrects a bug when using the prognostic closure (progsigma = true) to compute the q-tendency due to microphysics even if the diagnostic flags are false. It also optimizes the cloudbase massflux value for shallow and deep convection to work with the overall P8 physics suite.
New baselines are needed for the test: control_c384_progsigma
This update does not change the results in the control tests.
###issues
It addresses the issue: NCAR/ccpp-physics#960
###dependencies
NCAR/ccpp-physics#967