DO NOT MERGE. Just to see diffferences. #4
Closed
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
…en_unit_conversions
… into feature_capgen_unit_conversions
…al dimension name
… into feature_capgen_unit_conversions
…ature_capgen_unit_conversions
… into feature_capgen_unit_conversions
…be consistent with how the DEBUG checking is implemented in PR NCAR#512
… capgen_in_scm
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
… are slices of a n+1 dimensional array have wrong allocation) (NCAR#600) ## Description 1. Bug fix in `scripts/mkstatic.py`: define `dim_string_allocate` so that temporary (group cap) variables of rank `n` that correspond to a slice of an `n+1` dimensional array have the right dimensions in the assignment calls and subroutine call lists. See the inline documentation added in `mkstatic.py` around line 1476 for more information. 2. Addition of test `test_prebuild/test_unit_conv` to test for the above and to test for the `capgen` issue reported in NCAR#594 (Unit conversion bug when variable is used by two different schemes with different units). Note that `ccpp_prebuild.py` did not suffer from this bug, but I wanted to make sure that we test for it. 3. In the development of the new test `test_unit_conv`, I discovered that we should declare all incoming host variables in the group caps as `target`. This is because it is tricky in prebuild to determine that a parent variable `bar` needs to have the `target` attribute in case a slice of it has the active attribute. This is a bit of an edge case, but I believe this additional attribute is safe. I will run full UFS RTs with this PR to check if the results are b4b or if the addition of `target` causes the compiler to optimize differently. 4. Minor updates to `test_prebuid/{test_blocked_data,test_chunked_data}`: fix wrong name of subroutines in diagnostic error messages, set thread number and thread counter to safe values when running over the entire domain.
## 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 NCAR#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 NCAR#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 NCAR#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 NCAR#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 NCAR#594 Co-authored-by: Dom Heinzeller <[email protected]>
…erical dimensions in debug checks.
… capgen_in_scm
Update `main` with latest commits from `develop`
…20241205 Update develop from main 2024/12/05
…ts (NCAR#617) **This PR affects ccpp-prebuild only. It can be merged into develop (or main), but it must come to main as soon as possible for use in UFS/SCM.** This PR adds workarounds for handling optional arguments the right way (finally!) in `scripts/ccpp_prebuild.py` and `scripts/mkcap.py`. This update is already in use in NEPTUNE and is required for @dustinswales' work to update/revert the optional arguments in ccpp-physics in the UFS and the SCM. The workaround for `ccpp-prebuild` allows us to treat only those arguments as optional that are truly optional for a CCPP scheme. In the past, any argument that was conditionally allocated by any of the host models had to be declared as optional, even if it was required by the physics. User interface changes?: Yes and No. This can be merged without making any changes (it won't break the previous functionality where any conditionally allocated variable had to be declared as optional in the physics). But it will allow to declare many CCPP physics variables as non-optional if they aren't really optional. This finally resolves NCAR#566 (by making ccpp-prebuild behave the same as capgen, which is the correct way to handle optional arguments). Testing: test removed: none unit tests: all pass system tests: all pass manual testing: implemented and tested thoroughly in NEPTUNE
Added test using a DDT host object to pass information Fix problems so that test passes Improve formatting for readability User interface changes?: No Fixes: NCAR#589 Testing: test removed: None unit tests: Pass system tests: Pass, added DDT host object test manual testing: Ran doctests, examined generated code for system tests --------- Co-authored-by: Steve Goldhaber <[email protected]> Co-authored-by: Dom Heinzeller <[email protected]>
PR to address issue with variable intents *across* groups. **Description** - Current behavior for variables across groups: the intent for the first group is used as the "truth". - If variable "foo" is intent "out" in Group A and intent "in" in Group B, "foo" will be added to the variable dictionary used by the host as intent "out", which then the host assumes means that the framework will handle it - Updated behavior for variables across groups: adjust the intent of the existing variable to "inout" if a conflict arises across groups (or suites) User interface changes?: No Testing: test removed: N/A unit tests: Updated capgen & ddt test - variable intents across suites also affected; PASS system tests: all PASS manual testing: Ran in CAM-SIMA
…ost cap call string for array references.
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.
[ 50 character, one line summary ]
[ Description of the changes in this commit. It should be enough
information for someone not following this development to understand.
Lines should be wrapped at about 72 characters. ]
User interface changes?: [ No/Yes ]
[ If yes, describe what changed, and steps taken to ensure backward compatibilty ]
Fixes: [Github issue #s] And brief description of each issue.
Testing:
test removed:
unit tests:
system tests:
manual testing: