Skip to content

RRTMGP w/ GFS SDFs + some houskeeping#181

Merged
grantfirl merged 54 commits into
NCAR:masterfrom
dustinswales:dtc/develop
Aug 11, 2020
Merged

RRTMGP w/ GFS SDFs + some houskeeping#181
grantfirl merged 54 commits into
NCAR:masterfrom
dustinswales:dtc/develop

Conversation

@dustinswales
Copy link
Copy Markdown
Member

This PR contains the necessary changes to work w/ ccpp-physics PR#446 (NCAR/ccpp-physics#446).

Similar to that PR, there were some changes to the names of the radiation heating rates variables, so there is some cleaning up in GFS_typedefs addressing this.

Comment thread scm/src/GFS_typedefs.F90 Outdated
@@ -1850,12 +1850,6 @@ module GFS_typedefs
real (kind=kind_phys), pointer :: cld_resnow(:,:) => null() !< Cloud snow effective radius
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@dustinswales I don't see these same changes in NCAR/fv3atm#46. Were they previously committed there? We try to keep the two GFS_typedefs as close to each other as possible.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@grantfirl
You are correct. I did some cleanup in here to the RRTMGP interstitial fields. I will do the same on the fv3atm side.

Comment thread .gitmodules Outdated
@@ -4,5 +4,5 @@
branch = dtc/develop
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

After testing, we'd like to see this reverted.

@grantfirl grantfirl changed the base branch from dtc/develop to master May 4, 2020 23:21
@dustinswales
Copy link
Copy Markdown
Member Author

SCM results, both SDFs, twpice configuration. RRTMGP (blue), RRTMG (red)
scm_GvGP_v15p2.pdf
scm_GvGP_v16beta.pdf

@grantfirl
Copy link
Copy Markdown
Collaborator

@grantfirl
I need to revisit it this. I will cleanup and update this afternoon.

OK, let me know if you need anything. I'm happy to help, since this is required to have the master SCM code work with ccpp-physics/master.

@dustinswales
Copy link
Copy Markdown
Member Author

@grantfirl
I updated to the NCAR master physics and added in some infrastructure to the SCM side, but I cannot get the prebuild to work.

@grantfirl
Copy link
Copy Markdown
Collaborator

@grantfirl
I updated to the NCAR master physics and added in some infrastructure to the SCM side, but I cannot get the prebuild to work.

OK, I'll pull in your changes and see what I can do. If I find something that I can fix, I'll submit a PR from my fork to yours. I'm also just about ready to merge in #191, so I'll update your branch for that too. It shouldn't affect anything from your PR since it's just adding new cases.

@grantfirl
Copy link
Copy Markdown
Collaborator

@dustinswales A quick update: starting from this branch, I now have a branch that is working OK. I'm building/compiling one last time to double-check, and then I'll submit a PR from my branch to this one for you to review. Where necessary, I took my lead from the associated changes in FV3 (like SDFs and ccpp_prebuild_config.py). You may want to pay particular attention to changes in those to make sure that I updated them as you intended.

@grantfirl
Copy link
Copy Markdown
Collaborator

If you accept the PR from my fork to yours, I'd like to have you remove the Apollo setup script before I accept/merge. I think that the code managers want to keep only setup scripts for officially-supported machines.

@dustinswales
Copy link
Copy Markdown
Member Author

@grantfirl
I accepted your PR. Builds/runs, everything works. I removed my local system configuration file.

Copy link
Copy Markdown
Collaborator

@grantfirl grantfirl left a comment

Choose a reason for hiding this comment

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

Thanks @dustinswales, approved. I think that this is good to merge on my side. I'll ask @climbfuji if he wants to take a look before merging.

Copy link
Copy Markdown
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

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

Two minor comments, but probably just my lack of understanding.

Comment thread ccpp/suites/suite_SCM_GFS_v15p2_ACM_ps.xml
Comment thread scm/src/GFS_typedefs.F90 Outdated
!
if (Model%do_RRTMGP) then
if (Model%use_LW_jacobian) then
!Interstitial%fluxlwUP_jac = clear_val
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why commented out? Same for line 6994.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@dustinswales I am curious about this too.

@grantfirl
Copy link
Copy Markdown
Collaborator

I'll go ahead and merge. If @climbfuji would rather me remove the ACM SDF altogether, I can do that in a followup.

@grantfirl grantfirl merged commit 486fd92 into NCAR:master Aug 11, 2020
dustinswales pushed a commit to dustinswales/ccpp-scm that referenced this pull request May 16, 2022
RRTMGP w/ GFS SDFs + some houskeeping
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.

3 participants