Update main with latest commits from develop
#615
Merged
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
gold2718
approved these changes
Nov 21, 2024
|
I started testing this in NEPTUNE - should have the results by the end of today |
climbfuji
approved these changes
Nov 26, 2024
|
Note. Once this PR is merged, I will send two small changes from the NEPTUNE fork back to main that will allow us (@dustinswales and @climbfuji) to redo the optional arguments for UFS, NEPTUNE, ... - this time we'll get it right! |
peverwhee
approved these changes
Nov 27, 2024
|
@dustinswales @mkavulich Shall I run SCM RTs? |
I've run prebuild with this PR and am building locally on my Mac. Once that succeeds, I'll start a PR into ccpp-scm and run RTs. |
SCM RTs running: NCAR/ccpp-scm#546 |
grantfirl
approved these changes
Dec 5, 2024
|
Time to merge! |
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: Passed ✅
CAM-SIMA RTs: Passed ✅
NEPTUNE RTs: Passed ✅