-
Notifications
You must be signed in to change notification settings - Fork 19
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
Implementation of CCPP-ized tropopause_find #112
Implementation of CCPP-ized tropopause_find #112
Conversation
Bring dry adiabatic adjustment (dadadj) physics scheme into main branch
- Rename to tropopause_find, _init, _run routines - Remove most uses of physics_state and clean argument lists - Remove use statements for physconst - Comment out history addfld/outfld calls for CAM-SIMA
…lit to utilities module in CAM
…c to pcols (not ncol)
…ics_state removal
…s; clarify arguments in driver routine
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.
Great work on your first major CCPP-effort @jimmielin! I have several comments, but they should almost all relate to standard names or changes to code comments. Of course please let me know if you have any issues or concerns with anything I requested. Thanks!
…pdates and revised code comments, cleanup
Updated history field list with new CCPP field names:
|
tropopause_find/tropopause_find.meta
Outdated
dimensions = () | ||
intent = in | ||
[ tropp_p_loc ] | ||
standard_name = tropopause_air_pressure_from_climatology |
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.
This is the input climatological dataset from tropopause_read_file
. Does this standard name look fine? I am a little concerned it looks too similar to tropopause_air_pressure_from_climatological_method
, so I was thinking if it would be good to change to tropopause_air_pressure_from_climatology_dataset
.
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.
I like using tropopause_air_pressure_from_climatology_dataset
Thanks @nusbaume for your review comments. I have updated standard names and updated comments throughout to address your comments, and requested a re-review. Tests on Derecho show changes here are b4b with the original PR. There is one exception for |
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.
Thanks for implementing all of my change requests! There was one last comment change request I missed the first time, but otherwise everything looks good (so no need to request a re-review). Thanks!
Co-authored-by: Jesse Nusbaumer <[email protected]>
Thanks @nusbaume for the review! I've committed your suggested change.
|
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.
Impressive conversion. Great job!
We'll need to make sure this code is tested in ESCOMP/CAM regression tests before it can be committed into this repo.
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.
Minor mods requested
tropopause_find/tropopause_find.meta
Outdated
dimensions = () | ||
intent = in | ||
[ tropp_p_loc ] | ||
standard_name = tropopause_air_pressure_from_climatology |
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.
I like using tropopause_air_pressure_from_climatology_dataset
…set; address review comments
Thanks @cacraigucar, comment updated (also in CAM-SIMA) and standard name for |
This avoids a regression in CAM where tropZ is attempted to be interpolated when geopotential height is unavailable. This would not be an issue in CCPP as the framework would check for the fields.
I ended up adding Re-running the tests now to see if tests pass now with this change. |
Hi @cacraigucar I ran the regression tests with the CCPP-ized tropopause_find. Tests on Izumi NAG match pre-existing failures; GNU all passed as expected. All tests on Derecho match pre-existing failures except for the SMS
Upon further investigation of the When I ran this only Thanks! |
Companion PR: ESCOMP/atmospheric_physics#112 Note: I think I brought in more changes than necessary in `ncar_ccpp` due to the my `atmospheric_physics` branch being based off `development` rather than the released used in CAM-SIMA. I can do a rebase if necessary... ## Changes made to support CCPP-ized tropopause_find * Copied utility module `interpolate_data.F90` from CAM * Added utility module `tropopause_read_file.F90` to provide climatological tropopause data to tropopause_find from CAM-SIMA * `&tropopause_nl tropopause_climo_file` is read from `tropopause_read_file.F90` inside SIMA and not in the parameterization, to avoid passing this file name back and forth between the parameterization to CAM-SIMA * Adds `get_curr_calday()` result to new standard name variable `fractional_calendar_days_on_end_of_current_timestep` * Submodule update (possibly need a rebase?) I recommend manually merging in the history PR for testing. It might be easier to merge my branch (`jimmielin:hplin/tropopause_find`) into `peverwhee:history_court`.
Implements CCPP-compliant tropopause_find.
Major changes to code
This is my first time doing this, so please feel free to suggest alternative approaches!
:ncol
fields, ... was made.tropopause_find
accepted "primary" and "backup" methods for finding the tropopause. All the methods known to be used in CAM are now calculated at the same time in thetropopause_find_run
main driver routine; it calls the underlying logic as appropriate with and populates the appropriate standard name physics fields. Because the same physical quantities for tropopause level, height, pressure, temperature, etc. using different methods are used throughout the CAM physics and simultaneously output in the history tape, the same standard names with a suffix are used to differentiate between these quantities computed by different methods. e.g.,The naming should probably be changed! I'm open to suggestions.
tropopause_read_file
, which provides climatological data used as the default "backup" method to compute the tropopause (tropp_p_loc
andtropp_days
), has been moved to a utility module in CAM-SIMA to read, regrid, and provide this climatology to the CCPP-ized physics; it also removeslchnk
indexing since they're no longer used in CAM-SIMA.optional
) arguments for returningtropT
,tropZ
,tropP
(tropopause temperature, height, and pressure) which are no longer optional per the CAM-SIMA port guide.A "shim" in current CAM's
tropopause.F90
was made to use the CCPP-ized routines, but preserve all existing behavior and functionality (b4b) as expected in current CAM. In CAM-SIMA all of this are not needed.Smaller changes
get_curr_calday()
is now provided in the registry asfractional_calendar_days_on_end_of_current_timestep
Outstanding issues/notes
Tropopause_find cannot be tested using snapshots within CAM-SIMA: In regular CAM,
tropopause_find
itself does not update physics state/tendencies; its use in regular CAM is just providing the tropopause level, height, pressure, and temperature to the calling physics package; a final run oftropopause_output
will then write out to history the computed tropopause properties using the primary method. This means if snapshots cannot verify whattropopause_find
is doing within CAM-SIMA.How to make standard names for the same physical quantities computed using different methods that have to co-exist in one model run
Answer check
For existing CAM
QPC6
(which uses CLUBB that usestropopause_find
) answers match bit-for-bit for history (h0i
) and snapshot files before and afterclubb_tend_cam
. I can run more detailed regression tests for the CAM tag.For CAM-SIMA
Tricky way to verify answers
We can validate the answer fields of
TROP_P
,TROP_T
, andTROP_Z
can be against the output of CAM by running CAM-SIMA from a CAM snapshot and using history to write out these fields (instantaneously) at the first timestep. Note thatndens
has to be set to1
in CAM (fordouble
) output, and auser_set
snapshot should be taken for use beforetropopause_output
inphyspkg.F90
for use inncdata
.