MPAS-Dev / MPAS

Repository for private MPAS development prior to the MPAS v6.0 release.
Other
4 stars 0 forks source link

Remove the call to the cloud microphysics driver from the dynamics RK routine #1410

Open davegill opened 7 years ago

davegill commented 7 years ago

The cloud microphysics should be treated similarly to the other atmospheric physics schemes. The existing physical parameterization schemes each have a driver, and that collection of drivers is called by a physics driver.

  1. The new post-dynamics physics driver calls the driver for the microphysics.
  2. The call to the post-dynamics physics driver is in the same routine as the call to the pre-dynamics physics driver. No changes in results are expected. This is a small re-organization of the calling tree for microphysics.
Original calling structure
--------------------------
atm_do_timestep
   physics_driver     <------------
   atm_timestep                   |
      atm_srk3                    |
         driver_microphysics      | same routine,
                                  | mostly just renamed
New calling structure             |
------------------------          |
atm_do_timestep                   |
   physics_driver_before_dyn <-----
   atm_timestep
      atm_srk3
   physics_driver_after_dyn
      driver_microphysics

Four files were modified, three are Fortran source code contained within the core_atmosphere directory tree and a Makefile to modify the dependency structure noted in the new calling sequence.

src/core_atmosphere/mpas_atm_core.F

After this modification, atm_do_timestep calls two physics drivers, so an “only” clause for two routines was added from module mpas_atmphys_driver: physics_driver_before_dyn and physics_driver_after_dyn. The original call (to the now first) physics_driver has been renamed to physics_driver_before_dyn. After the existing call to atm_timestep, a call to physics_driver_after_dyn has been added.

src/core_atmosphere/dynamics/mpas_atm_time_integration.F

Within the atm_srk3 routine, the call to the microphysics driver was removed. The supporting pieces related to that cloud microphysics call were also removed from within this routine: configuration flags for microphysics and convection, pointer for Qv tendency for dynamics, and the entire code block to call the microphysics driver.

src/core_atmosphere/physics/Makefile

For the target src/core_atmosphere/physics/mpas_atmphys_driver.o, the list of dependencies was augmented with mpas_atmphys_driver_microphysics.o.

src/core_atmosphere/physics/mpas_atmphys_driver.F

Two types of modifications were implemented. The existing physics driver was renamed, and a few calls to utility routines now reflect the new subroutine name. The second physics driver was added, for a total of two drivers now within this module. The original subroutine have been renamed from physics_driver to physics_driver_before_dyn. The new subroutine is physics_driver_after_dyn. Currently this routine is only for cloud microphysics processes. Both of these routines are public, and both are called by atm_do_timestep. Since the original module had only one subroutine, the location of the description of the subroutines that were called by the physics_driver did not matter. Now with two routines, the descriptions of those called routines has been moved within physics_driver_before_dyn.

When the new new routine physics_driver_after_dyn was added, it was structured similarly to physics_driver_before_dyn with a couple of exceptions. First, the explicit allocates and deallocates for physics-sized arrays are still handled entirely within the microphysics driver itself, that was not modified. Second, the testing for the cumulus scheme for tendency contributions and positive definite checks for scalars are lifted directly from the original location in subroutine atm_srk3.

Test Results

For a 5-day, 60-km uniform mesh, using optimized ifort, the before vs after results are bit-for-bit. The bit-wise identical results are expected because this modification is only an infrastructure change to the calling structure hierarchy.

davegill commented 7 years ago

I do not have the little check box next to the PR, so I can't add a label. I would add "atmosphere".

Is there a problem with my permissions on this repository? I also see a message that says "Only those with write access to this repository can merge pull requests."

mgduda commented 7 years ago

@davegill Regarding permissions, I think only those on the atm-maintainers team can merge PRs; you may be only on the atm-developers team. It is pretty silly that a developer cannot add a label to their own PR.

mgduda commented 7 years ago

Regarding PR style, we don't use any GitHub buttons to merge PRs for MPAS. Instead, the first comment on a PR is used almost verbatim as the merge commit message. We'd generally like this to be about a paragraph in length. Further details about testing, etc. can be added to the PR in subsequent comments. As an example, here's what happens to be the most recent merge commit on the develop branch:

commit 6dac06c0a5e1b5681340826bbfc41ac1037a7c98
Merge: 960a648 3fa8266
Author: Michael Duda <duda@ucar.edu>
Date:   Tue Sep 12 17:07:09 2017 -0600

    Merge branch 'framework/log_write_IBM_error' into develop

    This merge fixes an unconventional usage of quotes in the mpas_io module that
    was causing a compile error for some versions of the IBM compiler. A quoted
    string was opened with a single-quote and closed with a double-quote, then
    joined with a string on a continuation line that was opened with a double-quote
    and closed with a single-quote. Now, single-quotes are used throughout the line.

    * framework/log_write_IBM_error:
      Correct quotation usage in log message in mpas_io.F

Note also that, because the branch name shows up in the merge commit message, we generally name our branches with the prefixes framework/, atmosphere/, ocean/, landice/, etc. so that anyone looking through the git log will know which part of MPAS is primarily affected by each merge. Also, for what it's worth, I don't know that anyone has named their branch using all caps.

mgduda commented 7 years ago

Also regarding style, we don't do "squash and merge" in MPAS, so each of the individual commits in a PR will ultimately be visible. The standard format is one line (hopefully less than 80 chars) summarizing the commit, then a blank line, then a description of the commit. Different versions of git apparently may either wrap or not wrap long lines in the log automatically. If you stop by my office, I can show you on my terminal what the commit message for da89bbe looks like. Accordingly, it's generally encouraged to keep the lines in a commit message under 80 characters.