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

Split run into phys_run1 and phys_run2 #253

Merged
merged 8 commits into from
Apr 12, 2024

Conversation

peverwhee
Copy link
Collaborator

@peverwhee peverwhee commented Mar 11, 2024

Summary

This PR brings in the necessary changes (and the necessary framework tag) to split up the physics run into phys_run1 (which calls the "physics_before_coupler" group) and phys_run2 (which calls the "physics_after_coupler" group).

Modifications

phys_comp.F90:

  • Create new timestep_final and timestep_init routines
  • Call cap run routine for "physics_before_coupler" in phys_run1
  • Call cap routine for "physics_after_coupler" in phys_run2
  • Verify that the user supplied valid group(s) [only physics_before_coupler and/or physic_after_coupler]

cam_comp.F90:

  • Add timestep_init and timestep_final routines so we can call those separately in atm_comp_nuopc
  • "use" the physics timestep variable from physics_types (the registry) instead of having it as a module variable here

atm_comp_nuopc.F90:

  • Call the new cam_timestep_init before cam_run1
  • Call the new cam_timestep_final before timestep advances

phys_comp.meta

  • File removed: metadata for horizontal_loop_begin and _end moved to physics_grid.F90; metadata for ccpp_error_message, ccpp_error_code, and timestep_for_physics moved to registry

write_init_files.py:

  • Add ccpp_error_message and ccpp_error_code to list of variables we never wish to read from a file

registry_v1_0.xsd

  • Update regex to enable "len=xxx" syntax

Testing

Confirmed no answer changes (via ncdata_check) for kessler or held suarez.

Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

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

Thanks for adding this functionality @peverwhee ! However I do wonder if now is a good time to cleanup some of the subroutines in phys_comp.F90, given that we seem to not really need many of the arguments being passed around anymore.

src/control/cam_comp.F90 Outdated Show resolved Hide resolved
src/data/write_init_files.py Outdated Show resolved Hide resolved
src/physics/utils/phys_comp.F90 Outdated Show resolved Hide resolved
src/physics/utils/phys_comp.F90 Outdated Show resolved Hide resolved
src/physics/utils/phys_comp.F90 Show resolved Hide resolved
src/physics/utils/phys_comp.F90 Outdated Show resolved Hide resolved
src/physics/utils/phys_comp.F90 Outdated Show resolved Hide resolved
src/physics/utils/phys_comp.F90 Outdated Show resolved Hide resolved
src/physics/utils/physics_grid.F90 Outdated Show resolved Hide resolved
@peverwhee peverwhee requested a review from nusbaume March 15, 2024 22:47
Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

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

Super-close! Just curious why you kept dtime_phys in the calling list for the various subroutines in phys_comp.F90? It doesn't appear to actually be used in many of them.

@peverwhee peverwhee requested a review from nusbaume March 18, 2024 18:03
Copy link
Collaborator

@nusbaume nusbaume left a comment

Choose a reason for hiding this comment

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

Thanks for tackling the extra clean-up! Everything looks good to me now.

@cacraigucar
Copy link
Collaborator

I did find this about groups in the https://github.com/NCAR/ccpp-scm. In the file scm/doc/TechGuide/chap_ccpp.tex, there is a section (note this is .tex, so there are some special characters):

The concept of grouping physics in the suite definition file (currently reflected in the \execout{<group name=XYZ''>} elements) enables groups'' of parameterizations to be called with other computation (perhaps related to the dycore, I/O, etc.) in between. In the suite definition file included in this release, three groups are specified, but currently no computation happens between \execout{ccpp_physics_run} calls for these groups. However, one can edit the groups to suit the needs of the host application. For example, if a subset of physics schemes needs to be more tightly connected with the dynamics and called more frequently, one could create a group consisting of that subset and place a \execout{ccpp_physics_run} call in the appropriate place in the host application. The remainder of the parameterizations groups could be called using \execout{ccpp_physics_run} calls in a different part of the host application code.

This reassured me that what we are doing in this PR is what we should be doing.

@cacraigucar
Copy link
Collaborator

Basic question: Should the timestep_init and timestep_final be controlled at the group level as well? I could see that a scheme that is in the "after_coupling" section might need something from the coupling that just happened in the dycore. Also, a "before_coupling" scheme might need to do a timestep_final that prepares something for the dycore.

@nusbaume @peverwhee

@peverwhee
Copy link
Collaborator Author

@cacraigucar I've thought about this as well but couldn't think of a reason why whatever needs to be done before the dycore (or after) couldn't be done as part of the run phase of that scheme. If we added some sort of group-specific version of timestep_final and init, we'd also need to rethink the naming.

src/control/cam_comp.F90 Outdated Show resolved Hide resolved
src/cpl/nuopc/atm_comp_nuopc.F90 Outdated Show resolved Hide resolved
src/physics/utils/physics_grid.F90 Show resolved Hide resolved
src/cpl/nuopc/atm_comp_nuopc.F90 Outdated Show resolved Hide resolved
src/cpl/nuopc/atm_comp_nuopc.F90 Show resolved Hide resolved
src/physics/utils/phys_comp.F90 Show resolved Hide resolved
src/physics/utils/phys_comp.F90 Show resolved Hide resolved
@peverwhee peverwhee merged commit a54b646 into ESCOMP:development Apr 12, 2024
6 checks passed
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