Skip to content
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
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

mkavulich
Copy link
Collaborator

Testing:
UFS RTs: Passed ✅
SCM RTs: Pending
CAM-SIMA RTs: Pending
NEPTUNE RTs: Pending

mkavulich and others added 13 commits July 11, 2024 13:43
…_" (#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]>
Copy link
Collaborator

@gold2718 gold2718 left a 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).

@climbfuji
Copy link
Collaborator

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants