-
Notifications
You must be signed in to change notification settings - Fork 12
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
Instantaneous CAM-SIMA history infrastructure #274
Conversation
Implement read of new history namelist Added unit tests for CAM history unit tests Added multiplier parser and utiliity unit tests Remove redundant filenames.F90 Finish some checking functions Added error strings to command parser functions First pass of HistoryConfig class Start on testing history configuration parsing Progress on hist config parsing Still working on new format, start with get_entry Finish parse_hist_config_file First pass at some AMWG history configuration Updated from focus meeting Fix location of history unit test sample files Take out debug exception Test passes but output file is not yet correct. Full test of flat user_nl_cam passes Updated namelist names to begin with hist_ Rename user_nl_cam history entries to begin with hist_ Progress on multi-level history config Reconfigured class structure, flat and multi tests pass Added history configuration parsing and writing to buildnml Add diagnostic element to registry Updated CMakeLists.txt for new share code file layout
…ing gravit standard name. Making etamid visible via metadata file.
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 requesting my review @peverwhee! This looks great, I only have a few minor comments, mostly on the total energy standard names that we discussed for a separate snapshot issue (ESCOMP/CAM#1141), since they were updated in this PR. My comments reflect the current state of the CAM snapshots that do not output the _using_dycore_formula
variants of te_ini
and te_cur
. #296 will make a more thorough fix.
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.
Looks a lot better! Just some last final suggestions and requests. Of course if you think these would be easier to push off until a later date (e.g. via a new issue) then just let me know. 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.
Looks great now! I have one last request, but it doesn't need a re-review. Thanks again for getting this implemented @peverwhee!
cime_config/hist_config.py
Outdated
return (None, errmsg) | ||
# end if | ||
# Check multiplier | ||
tokens = [x.strip() for x in str(entry.strip()).split('*')] |
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 think entry
is already a string, so no need to convert it here. Also I don't think there is any need to strip
twice:
tokens = [x.strip() for x in str(entry.strip()).split('*')] | |
tokens = [x.strip() for x in entry.split('*')] |
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.
cleaned up! 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.
Thanks for tackling this one @peverwhee! Looks good to me!
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 have not reviewed the actual code here (CAM history has always been pretty much a black box for me). My review mainly has been to use what has been implemented and make sure it works (as it does). Let me know if you want me to review the Fortran code, otherwise I am giving my approval (pending the changes I've requested for the submodules locations).
Tag name (required for release branches):
Originator(s): peverwhee, gold2718
Summary (include the keyword ['closes', 'fixes', 'resolves'] and issue number):
inverse_exner_function_wrt_surface_pressure
toreciprocal_of_dimensionless_exner_function_wrt_surface_air_pressure
Describe any changes made to build system:
A cime_config/hist_config.py: New namelist parsing for history configurations
M cime_config/buildlib: Add new history directories
M cime_config/buildnml: Incorporate hist_config.py
M cime_config/atm_in_paramgen: Incorporate hist_config.py
Describe any changes made to the namelist: Can now use CAM-SIMA history namelist - see https://escomp.github.io/CAM-SIMA-docs/usage/history/
List any changes to the defaults for the input datasets (e.g. boundary datasets): none
List all files eliminated and why:
D src/control/filenames.F90
-renamed to src/utils/cam_filenames.F90
List all files added and what they do:
A src/control/cam_physics_control.F90
-split cam_control_mod to avoid circular dependency
A src/utils/cam_filenames.F90
-renamed from control/filenames.F90
A src/history/cam_hist_file.F90
A src/history/cam_history.F90
A src/history/cam_history_support.F90
-core CAM-SIMA history infrastructure
A test/unit/test_hist_config.py
-test hist_config.py (namelist parsing)
List all existing files that have been modified, and describe the changes:
(Helpful git command:
git diff --name-status development...<your_branch_name>
)M src/control/cam_comp.F90
-add calls to new history infrastructure
M src/control/cam_control_mod.F90
-split to into cam_control_mod and cam_physics_control
M src/control/runtime_opts.F90
-add call to history namelist reader
M src/control/cam_initfiles.F90
-update use statement
M src/cpl/nuopc/atm_comp_nuopc.F90
-add rstwr and nlend to cam_timestep_final calls, add do_history_write logical
M src/data/registry.xml
M src/dynamics/se/dyn_comp.F90
M src/dynamics/mpas/dyn_comp.F90
-update exner and vertically_integrated_* standard names
M src/dynamics/utils/hycoef.F90
-enable history coordinates
M src/utils/cam_abortutils.F90
-fix bug in cam_register_open_file
M src/utils/cam_field_read.F90
-code cleanup
M src/utils/cam_grid_support.F90
-update to enable split history files
M src/utils/cam_pio_utils.F90
-only register created and opened files if we're writing to them
M src/utils/cam_time_coord.F90
M src/utils/time_manager.F90
-update use statement
M src/utils/string_utils.F90
-add functions date2yyyymmdd and sec2hms
M test/run_unit_tests.sh
-add new hist_config test
If there are new failures (compare to the
test/existing-test-failures.txt
file),have them OK'd by the gatekeeper, note them here, and add them to the file.
If there are baseline differences, include the test and the reason for the
diff. What is the nature of the change? Roundoff?
derecho/intel/aux_sima:
derecho/gnu/aux_sima:
If this changes climate describe any run(s) done to evaluate the new
climate in enough detail that it(they) could be reproduced: