-
Notifications
You must be signed in to change notification settings - Fork 64
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
Update main
with latest commits from develop
#615
Open
mkavulich
wants to merge
13
commits into
main
Choose a base branch
from
update_from_develop_2024-11-21
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…_" (#569) Remove hard-coded requirement in prebuild that suite definition file names begin with the literal string `suite_`. Includes back-compatibility logic to allow for the previous naming convention to continue to work.
Add capgen automated testing on PRs to "develop"
Use absolute paths for dependencies In order to make the dependencies in the CCPP datatable more usable, store absolute pathnames where possible. User interface changes?: Yes This may require a change in any build system that uses the dependencies. Fixes: #575 Testing: test removed: None unit tests: passed system tests: ? manual testing: ran Fortran tests --------- Co-authored-by: Steve Goldhaber <[email protected]>
Tiny bugfix for unit conversion edge case that was trying to convert "None" to "none" ## Description Convert "none" units to lowercase before comparing. User interface changes?: No Fixes: closes #567 Testing: - Updated var_compatability_test to include a "None" to "none" comparison Co-authored-by: Courtney Peverley <[email protected]>
…eta files (#581) Because of the change in directory structure for CCPP physics (ufs-community/ccpp-physics#99), there are now `.meta` files at different levels in the directory tree. The `ccpp_track_variables.py` script needs the location of these `.meta` files as an input argument to the script, but the call to `glob.glob` in the script does not use the `recursive=True` argument, so even if the user passes in the argument `-m './physics/physics/**/'` (which should include all subdirectories), the call to `glob.glob` only searches one level. Our simple test case only has `.meta` files at a single directory level, so we never caught this issue. Simply adding the `recursive=True` argument fixes this issue.
Update call_command function to include stderr output in CCPPError message. Right now, when xmllint-ing is done via xml_tools.py (validate_xml_file), the output of the linter is not returned. This PR adds the stderr output (if any) to the returned error message. PR also decodes error messages for cleaner output. User interface changes?: No Testing: test removed: N/A unit tests: Added new doctest to test error message system tests: N/A manual testing: Ran with/parsed new error messages within CAM-SIMA
## Overview This PR adds a new phase, *register*, that can be called by a host model and used by schemes to perform any set up that needs to happen BEFORE the grid is established. NOTE: this PR also *removes* the old `dynamic_constituent_routine` metadata implementation for runtime constituents. ## Description I have implemented it as an "optional" phase, by which I mean that it is not required that a host model call this phase (though I'm happy to be overruled!). As a result, the register phase does not change the CCPP "state" (but will produce an error if it is called after the `init` phase). More: ### Dynamic/run-time constituent handling: - If a scheme has run-time constituents, those shall be allocated, instantiated, and returned from the scheme's register phase. This metadata is required (the framework determines that there are runtime constituents from a scheme if there is a `ccpp_constituent_properties_t` variable required): ``` [ <unique dynamic constituent local name> ] standard_name = <some unique standard name> dimensions = (:) type = ccpp_constituent_properties_t intent = out allocatable = true ``` - The standard name doesn't really matter but MUST be different from other runtime constituent standard names in the scheme; it may be easiest to standardize this to something like `dynamic_constituents_for_<scheme>` - The framework will then compile all scheme constituents into module-level variables in the host cap called `<suite>_dynamic_constituents`, which are then used to pack and initialize the module level constituents object `<host>_constituents_obj`. - If there are no dynamic constituents registered by any schemes within a suite, that suite's dynamic constituents array is allocated to 0. *Generated host cap code examples* 1. Multiple schemes have dynamic constituents: ``` subroutine test_host_ccpp_physics_register(suite_name, errmsg, errflg) use ccpp_cld_suite_cap, only: cld_suite_register character(len=*) :: suite_name character(len=512) :: errmsg integer :: errflg type(ccpp_constituent_properties_t),allocatable :: dyn_const(:) type(ccpp_constituent_properties_t),allocatable :: dyn_const_ice(:) integer :: num_dyn_consts integer :: const_index errflg = 0 errmsg = "" if (trim(suite_name) == 'cld_suite') then call cld_suite_register(errflg=errflg, errmsg=errmsg, dyn_const=dyn_const, & dyn_const_ice=dyn_const_ice) allocate(cld_suite_dynamic_constituents(0+size(dyn_const)+size(dyn_const_ice))) ! Pack the suite-level dynamic, run-time constituents array num_dyn_consts = 0 do const_index = 1, size(dyn_const) cld_suite_dynamic_constituents(num_dyn_consts + const_index) = dyn_const(const_index) end do num_dyn_consts = num_dyn_consts + size(dyn_const) deallocate(dyn_const) do const_index = 1, size(cld_suite_dynamic_constituents) call cld_suite_dynamic_constituents(const_index)%standard_name(stdname, & errcode=errflg, errmsg=errmsg) end do do const_index = 1, size(dyn_const_ice) cld_suite_dynamic_constituents(num_dyn_consts + const_index) = & dyn_const_ice(const_index) end do num_dyn_consts = num_dyn_consts + size(dyn_const_ice) deallocate(dyn_const_ice) else write(errmsg, '(3a)')"No suite named ", trim(suite_name), "found" errflg = 1 end if end subroutine test_host_ccpp_physics_register ``` 2. No schemes have dynamic constituents: ``` subroutine test_host_ccpp_physics_register(suite_name, errmsg, errflg) use ccpp_ddt_suite_cap, only: ddt_suite_register use ccpp_temp_suite_cap, only: temp_suite_register character(len=*) :: suite_name character(len=512) :: errmsg integer :: errflg errflg = 0 errmsg = "" if (trim(suite_name) == 'ddt_suite') then call ddt_suite_register(errflg=errflg, errmsg=errmsg) ! Suite does not return dynamic constituents; allocate to zero allocate(ddt_suite_dynamic_constituents(0)) else if (trim(suite_name) == 'temp_suite') then call temp_suite_register(errflg=errflg, errmsg=errmsg, config_var=config_var) ! Suite does not return dynamic constituents; allocate to zero allocate(temp_suite_dynamic_constituents(0)) else write(errmsg, '(3a)')"No suite named ", trim(suite_name), "found" errflg = 1 end if end subroutine test_host_ccpp_physics_register ``` ### Misc notes Since this phase is called before the grid is initialized, variables are not allocated at this time (that still happens in `init`) and no variables with horizontal and vertical dimensions can be passed in. ## UI Changes User interface changes?: Yes, but they're optional If a host model wishes to utilize schemes' register phases, they must add a call to `<host_model>_ccpp_physics_register(suite_name, errmsg, errflg)` ## Testing test removed: removed unit tests for dyn_const_routines (old implementation of runtime constituent handling) - all pass unit tests: Removed old dynamic constituents testing - all pass system tests: Updated capgen and advection tests to include register phases (with and without dynamic constituents) - Also updated advection test CMakeLists to first run a version with dynamic constituents in the wrong phase and have an expected error - This is perhaps not the best way to test this, but it's what I came up with manual testing: Fixes: closes #572 --------- Co-authored-by: Courtney Peverley <[email protected]> Co-authored-by: Courtney Peverley <[email protected]> Co-authored-by: Courtney Peverley <[email protected]>
Add missing xmllint dependency for capgen tests Adds the `libxml2-utils` dependency to allow full capgen tests to run without printing an error message about a missing dependency User interface changes?: No Fixes: closes #601 (by adding missing dependency) Testing: test removed: None unit tests: CI run system tests: N/A manual testing: N/A Co-authored-by: Dom Heinzeller <[email protected]>
Fixes a couple of small bugs in the constituents object 1. Add missing units field to equivalence check and constituent copy 2. Add missing fields to instantiate call 3. Parse mixing ratio type from standard name correctly (if "mixing_ratio_type" not provided to instantiate) 4. Add check of errflg in register to return before allocating if there's an error User interface changes?: No Fixes #587 Testing: test removed: N/A unit tests: All pass system tests: All pass; modified advection test to check new instantiate fields manual testing: Run w/ register phase in CAM-SIMA --------- Co-authored-by: Courtney Peverley <[email protected]>
Adds constituent tendency array and enables the use of individual constituent tendency variables. In order to facilitate time-splitting, this PR adds/enables the following: - ccpp_constituent_tendencies: standard name for constituent tendencies array - standard names of the form "tendency_of_CONSTITUENT" now handled by the framework - must also include "constituent = True" in metadata for the tendency Testing: unit tests: Added doctests to new check_tendency_variables function in host_cap.F90 system tests: Modified advection_test to use tendency variable and tendency array
Fixes bug introduced by me in #608. Use variable errflg name instead of "errflg" explicitly
Modifications made to handle local variables needed for variables transforms at the Scheme level. Previously the Group would manage the local variable Fixes #594 Co-authored-by: Dom Heinzeller <[email protected]>
mkavulich
requested review from
climbfuji,
gold2718,
dustinswales,
mwaxmonsky,
peverwhee and
grantfirl
as code owners
November 21, 2024 17:00
gold2718
approved these changes
Nov 21, 2024
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 looks fine to me (the update_from_develop_2024-11-21 branch is identical to the develop branch).
I started testing this in NEPTUNE - should have the results by the end of today |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Testing:
UFS RTs: Passed ✅
SCM RTs: Pending
CAM-SIMA RTs: Pending
NEPTUNE RTs: Pending