ncar / ccpp-framework Goto Github PK

As of 20180713, the NEMSfv3gfs-CCPP integration does not abort if a call to a specific scheme or group via ccpp_physics_run(scheme_name="..") (similar for group name) is not found.

The correct way to handle this is to return with an error from ccpp_physics_run and catch this error in the calling host model.

Since I do not know where the problem lies (ccpp-framework or FV3), the issue is recorded here.

capgen could take an option to insert host-model specific timing calls which would then be inserted in the generated code. Some initial thoughts and questions:

• The option would have to specify start and stop functions along with a module.
• The details could be included in the type = host ccpp-data-table at which point the capgen option only needs to be a boolean.
• The timers used by CESM allow nesting meaning that calls could be inserted at both the group level and at the individual scheme-call level. @climbfuji, @davegill, is this true of timers used in your models?
• The timers used by CESM take a single character string argument which is how entry and exit are marked. @climbfuji, @davegill, what is the interface to timers used in your models?

Updates of homebrew have removed the gcc/gfortran version used by travis: https://travis-ci.org/NCAR/ccpp-framework/jobs/605746515?utm_medium=notification&utm_source=email

Travis build on linux is still working as expected.

Upon finding a unit conversion mismatch, the exception: "Error, automatic unit conversion from X to Y not implemented" is raised without telling the user in which file and for which variable the mismatch occurs.

Depending on the length of the standard names and the variable from which to grab the data, some of the auto-generated ccpp_field_add statements are too long. While this is tolerated by the Intel and GNU compilers (but not the PGI compiler), it is a violation of the Fortran standard. We need to add a line wrap mechanism for these, similar to what is done for calling the physics schemes from the auto-generated caps.

In order to properly handle physics variables, a host model should be able to request the variables required to call any group in a CCPP suite.
This information may be needed by the host model at build time or at run time.

capgen currently always overwrites all generated code (caps, ccpp_kinds.F90). By default, it should only overwrite an existing file if that file has been modified. Here is the proposed solution.

• Generate all files to a temporary directory (e.g., if the capgen directory is ccpp, generate into ccpp_tmp).
• Only copy files from the temporary directory if they do not exist in the configured destination or have changed.
• Remove the temporary directory.

In addition, capgen will have a new switch, --force, which reproduces the current behavior (i.e., regnerate all files). This could be implemented as first removing the existing directory or by specifying that the temporary directory is the same as the existing directory (my preference as it would not overwrite anything unless the analysis phase is successful).

mfshalcnv.f is temporarily left in SCHEME_FILES in ccpp_prebuild_config_SCM.py because shallow convection specific interstitial code is still in mfshalcnv.f. This can be removed when the interstitial code is put in the correct place.

In order to both be able to check

use, intrinsic :: iso_fortran_env, only: r8 => REAL64

kind_phys

The unit testing framework is only built in the standalone configuration of ccpp-framework, mainly because of the difficulties to determine the correct path to the CCPP prebuild script. The prebuild script must be run as part of the cmake process to generate the caps that get compiled afterwards and that are needed for the tests.

It is to be discussed whether this approach is ok, i.e. whether it is reasonable to build the testing framework only in the standalone build. If not, there are workarounds like a symbolic link to ccpp_prebuild.py inside the directory where the cap-generation is triggered by CMakeLists.txt.

The new ccpp_capgen run-time interface, ccpp_physics_suite_variables, returns the standard name of requested variables. This interface should be enhanced to include all the relevant information about a variable. The proposal is to have the framework create a Fortran derived type to communicate this information:

   type, public :: ccpp_suite_variable_t
      character(len=max_stdname_len) :: standard_name = ''     ! standard name
      character(len=5)               :: intent = ''            ! intent
      logical, private               :: advect = .false.       ! is advected
      logical, private               :: wet_scavange = .false. ! wet deposited
      logical, private               :: dry_scavange = .false. ! dry deposited
   contains
      procedure :: is_advected      => ccpp_var_is_advected
      procedure :: is_input         => ccpp_var_is_input       ! in or inout
      procedure :: is_output        => ccpp_var_is_output      ! out or inout
      procedure :: is_wet_scavanged => ccpp_var_is_wet_dep
      procedure :: is_dry_scavanged => ccpp_var_is_dry_dep
   end type ccpp_suite_variable_t

The ccpp_physics_suite_variables inspection routine would allocate and return an array of these structures.
It will be easy to add new information to this type as the need arises. Modifications should be backwards compatible.
Thoughts? @areinecke?

In order for the CCPP framework to function as a library that can be used with many host models, it should not require information for any host model to be in its repository.

CGD systems have libXML2 installed via Anaconda and CMake is not finding it.
I suggest adding some documentation to the build instructions on how to work around issues such as this. For instance, in my case adding

-DPC_LIBXML_INCLUDEDIR=/usr/local/anaconda/include/libxml2 -DPC_LIBXML_LIBDIR=/usr/local/anaconda/lib

to the cmake command line fixed this issue.
This may help users with non-standard systems who are not familiar with the inner workings of CMake.

Unit test for capgen's parse_metadata_file does not detect missing intent statement.

In order to facilitate interoperability with schemes and host models which use derived data types, I think we need some clear rules for how DDTs are used in the framework. I propose a set of rules below.

• Any DDT used by a host model or a scheme must have a metadata header and be presented to the framework (i.e., ccpp_capgen) before any use of that DDT. This will allow the system to know the type name and containing module for scheme cap generation.
• Any component of a DDT used by a scheme must be documented in the DDT header. This implies that components which are not used by any scheme do not need to be included in the header. This will allow for type and standard_name comparison.
• A scheme dummy argument which is a DDT (as opposed to a component of a DDT) will have a more specific documentation in the metadata, in particular, not using the units entry which is not relevant for a DDT. The type keyword will be either type or class (matching the dummy argument) and the kind keyword will be the name of the DDT. An example is below.

[ Grid ]
type = type
kind = GFS_grid_type
standard_name = fv3_gfs_grid_type
long_name =  Fortran DDT containing FV3-GFS grid and interpolation related data 
intent = in

With #70, the _init and _finalize routines of each entry in the SDF are run in the order they appear in the SDF. In case a scheme is listed twice, also the _init and _finalize routines are run twice - which they shouldn't.

In order to control the complexity of suites, we could allow a <suite> keyword in the suite definition file (SDF) in order to allow for some modularity. For instance, if a suite needs to run a chemistry suite in the middle of a group, it could just add that suite in the desired place. Moving chemistry then becomes as simple as moving a single line in the SDF.
The framework would treat a <suite> keyword much like a C pre-processor #include statement -- it would simply (recursively) add the entries of the referenced SDF as if they were part of the main SDF being processed.
Post-framework displays of the suite would act as if the suite was monolithic (no indication that there were sub-SDFs) or could include a comment to mark those locations.
Example with two SDFs

<suite name="B" version="1.0">
   <group name="G">
      <scheme>A</scheme>
      <scheme>S</scheme>
   </group>
</suite>

<suite name="A" version="1.0">
   <group name="phys">
      <scheme>B</scheme>
      <scheme>L</scheme>
      <suite name="B" group="G" />
      <scheme>T</scheme>
   </group>
</suite>

would be equivalent to:

<suite name="A" version="1.0">
   <group name="phys">
      <scheme>B</scheme>
      <scheme>L</scheme>
      <scheme>A</scheme>
      <scheme>S</scheme>
      <scheme>T</scheme>
   </group>
</suite>

Another use would be to group suites together but not merge them. In this case, we would have something like:

<suite name="physics" version="1.0">
   <group name="phys">
      <scheme>B</scheme>
      <scheme>L</scheme>
      <scheme>T</scheme>
   </group>
</suite>

<suite name="dycore" version="1.0">
   <group name="fast">
      <scheme>H</scheme>
      <scheme>R</scheme>
   </group>
</suite>

<suite name="UFS" version="1.0" merge="FALSE">
      <suite name="physics" />
      <suite name="dycore" />
</suite>

In this case, two suite caps would be generated which could be called from different parts of the executables acting as separate host models. If we adopt this approach, the first example above might need a merge="TRUE" attribute.

Thoughts?

This may be a moot point with capgen.py, but ccpp_prebuild.py is still requiring a blank doxygen line (!! [blank]) at then end of where the metadata table used to be in the source code, i.e. it still expects it after the !! \htmlinclude line. This seems unnecessary now.

The standalone configuration of ccpp-framework, i.e. when cmake is called directly and not through a host model's CMakeLists.txt file, should be revisited and optimized. In particular, we should discuss whether the name of the project that identifies the type of build should be "Standalone" instead of "Unknown" which is the current default.

ccpp_prebuild.py currently throws an exception when the ending line of a module=scheme are not found. This happens at a later stage when the CCPP entry points for the module are scanned and the keys 'endline' is not in the dictionary.

    for module_name in registry.keys():
        startline = module_lines[module_name]['startline']
        endline = module_lines[module_name]['endline']
        line_counter = 0
        in_subroutine = False

This happens for example when the actual Fortran code is wrong, i.e. the end module this_module_name statement is missing. In this case, the parser should report the error earlier on with the offending filename and module name.

When building the CCPP framework as part of FV3 current trunk, the following warning is generated:

CMake Warning:
Manually-specified variables were not used by the project:

CCPP_MKCAP

Similar warnings may be generated when the framework is built as part of SCM.

Currently CCPP does not have the capability to run the scheme _init and _finalize routines as part of the init and finalize steps. We need to add this, in particular to support the implementation of the necessary GFDL microphysics initialization in CCPP.

The capgen code generator sometimes emits an incorrect intent dummy argument property (e.g., for a host-model loop variable).
Discovered by @mcgibbon.

Check if there are any hotfixes and updates in the ufs release branch for ccpp-framework that need to be brought back to master.

The current CI fails, because (a) test #3 throws an intentional error that it didn't do before and the assert statement was not updated, and (b) test #8 fails because the test target (test_check_cap.F90) does no longer exist. (a) should be fixed by using the right assertion statement, (b) by auto-generating test_check_cap.F90 using the current/new method (running ccpp_prebuild.py).

In the new metadata convention, array variables must document their dimensions. For example:

[o3vmr]
  standard_name = o3_vmr_col
  long_name = O3 volume mixing ratio column
  units = mole/mole
  dimensions = (vertical_layer_dimension)
  type = real | kind = kind_phys
  intent = in

Should the framework require that dimension variables (vertical_layer_dimension in this case) be included in the same metadata table (i.e., also be an argument to the scheme)?

The top-level README.md is seriously outdated and several instructions in there are no longer valid or don't work anymore (e.g. validation of suite definition files).

In order to both be able to check type compatibility and to be able to generate different type sizes for (e.g.) kind_phys, I think we need to standardize types and use aliases to be able to cross check without undue restrictions.

At the moment, the only flexible type I think we need is kind_phys which could default to REAL64 and could be specified on the ccpp_capgen command line (e.g., --kind_phys=REAL32).

machine.F90 would be auto-generated with a definition for kind_phys and translations to other names would be made in Fortran modules. for instance:

use ISO_FORTRAN_ENV, only: r8 => REAL64
use ISO_FORTRAN_ENV, only: kind_grid => REAL64

or

use machine, only: r8 => kind_phys
use machine, only: kind_grid => kind_phys

or

use ISO_FORTRAN_ENV, only: r8 => REAL64
use machine, only: kind_grid => kind_phys

The type checker would process use lines for either ISO_FORTRAN_ENV or machine and internally use standard Fortran types (e.g., REAL64 or REAL32).

If a scheme or suite has dummy arguments with the same name (as the host or another scheme) but a lower rank, the framework should generate the correct glue code where possible.
Example case:
A scheme's dummy arguments have a lower rank than used elsewhere in the suite (e.g., no vertical_dimension dimension). The framework could generate a loop to call this scheme or set of schemes.
Of course, the scheme(s) in question must not have the missing dimension in any of their dummy arguments for the analysis to work.

If a user enters an invalid table header, like [ccpp-farg-table], instead of [ccpp-arg-table] for the first meta table, the error is not caught. If the erroneous table header is used for the second table, the appropriate exception is raised.

In order to convert CCPP physics to the new metadata standard, we need a conversion tool plus code that parses files with the new metadata headers. Some issues regarding the conversion are:

• standard names will be converted to lower case and have dashes and periods converted to underscores
• questionable units (e.g., blank, 'none', 'count', 'flag', 'DDT' vs. 'various') are not being converted
• The first three dimensions default to 'horizontal_dimension', 'vertical_dimension', and 'number_of_tracers'
• Old metadata tables with non-standard usage of initial exclamation marks will not be converted

Using a specified Fortran code as input, generate a template file for the metadata for the requested routines (there may be routines which are internal to the module which do not need metadata). Create and fill in the local variable name, intent, dimensions (if needed). Create empty placeholders for other mandatory fields (such as standard name and units)

In order to support more functionality and to enhance support for host model build systems, the capgen interface will be enhanced as described below.

The new interface to ccpp_capgen is:

usage: ccpp_capgen.py [-h] --host-files <host files filename> --scheme-files
                      <scheme files filename> --suites <Suite definition
                      files> [--preproc-directives VARDEF1[,VARDEF2 ...]]
                      [--ccpp-datafile <filename for capgen output>]
                      [--output-root <directory for generated files>]
                      [--host-name HOST_NAME] [--nuopc-cap-name NUOPC_CAP_NAME]
                      [--kind-phys kind_phys]
                      [--generate-docfiles HTML | Latex | HTML,Latex]
                      [--verbose] [--clean]

Create CCPP parameterization caps, host-model interface code,
physics suite runtime code, and CCPP framework documentation.

optional arguments:
  -h, --help            show this help message and exit
  --host-files <host files filename>
                        Comma separated list of host filenames to process
                        Filenames with a '.meta' suffix are treated as host model metadata files
                        Filenames with a '.txt' suffix are treated as containing a list of .meta
                        filenames
  --scheme-files <scheme files filename>
                        Comma separated list of scheme filenames to process
                        Filenames with a '.meta' suffix are treated as scheme metadata files
                        Filenames with a '.txt' suffix are treated as containing a list of .meta
                        filenames
  --suites <Suite definition file(s)>
                        Comma separated list of suite definition filenames to process
                        Filenames with a '.xml' suffix are treated as suite definition XML files
                        Other filenames are treated as containing a list of .xml filenames
  --preproc-directives VARDEF1[,VARDEF2 ...]
                        Proprocessor directives used to correctly parse source files
  --ccpp-datafile <filename for capgen output>
                        Filename containing XML describing the CCPP framework analysis and
                       code generation
  --output-root <directory for generated files>
                        directory for generated files
  --host-name HOST_NAME
                        Name of host model to use in the CCPP API
  --nuopc-cap-name NUOPC_CAP_NAME
                        Name of a NUOPC component cap to use in the CCPP API
  --kind-phys kind_phys
                        Data size for real(kind_phys) data
  --generate-docfiles HTML | Latex | HTML,Latex
                        Generate LaTeX and/or HTML documentation
  --verbose             Log more activity, repeat for increased output
  --clean <ccpp datafile>
                        Remove files created by the run of this script indicated in the CCPP datafile,
                        then exit

A key service to the host model build system is the ccpp datafile. It contains the following information:

• A list of the full pathname for each Fortran file generated by the CCPP Framework. This list can be used by a build system (e.g., CMake) to generate a target source list and dependencies.
• A list of metadata files used by the CCPP Framework to implement all of the requested suites. Any metadata file passed to the framework which is not used in any specified suite will be be omitted.
• A list of fields which the host model must manage (e.g., because they are used by more than one suite but not provided by the host model). This section will also include the name of the Fortran module which contains definitions of these fields
• A list of fields provided by the host model which are used by the specified suites
• A dataflow graph which can be used to trace the use and modification of any field as it passes through any of the specified suites. This graph can also be used to produce display of a suite as (possibly) modified by the CCPP Framework
• A list of fields and metadata for any field whose state needs to be saved to and restored from a host-model restart file

The current Fortran type declaration parser Ftype_type_decl is used by ccpp_prebuild.py to parse all type declarations. For type declarations that are followed by a comment on the same line, the parser returns a wrong name of the type. For

type foo      ! this is a comment
  integer :: bar
end type foo

the parser returns a type_name foo ! this is a comment.

This problem can be circumvented in ccpp_prebuild.py by stripping out all trailing comments when parsing Fortran code for identifying module and data structures (see PR #205).

When automatic unit conversions are performed with ccpp_prebuild.py, no log message is produced. This makes it hard for people to understand what is going on. The log message should be at info level and added in the following two functions in https://github.com/NCAR/ccpp-framework/blob/master/scripts/mkcap.py

    def convert_to(self, units):
        """Generate action to convert data in the variable's units to other units"""
        function_name = '{0}__to__{1}'.format(string_to_python_identifier(self.units), string_to_python_identifier(units))
        try:
            function = getattr(unit_conversion, function_name)
        except AttributeError:
            raise Exception('Error, automatic unit conversion from {0} to {1} for {2} in {3} not implemented'.format(self.units, units, self.standard_name, self.container))
        conversion = function()
        self._actions['out'] = function()

    def convert_from(self, units):
        """Generate action to convert data in other units to the variable's units"""
        function_name = '{1}__to__{0}'.format(string_to_python_identifier(self.units), string_to_python_identifier(units))
        try:
            function = getattr(unit_conversion, function_name)
        except AttributeError:
            raise Exception('Error, automatic unit conversion from {1} to {0} for {2} in {3} not implemented'.format(self.units, units, self.standard_name, self.container))
        conversion = function()
        self._actions['in'] = function()

@tanyasmirnova @haiqinli

The ccpp_xml_parse_fptr subroutine returns an incorrect error status.

This is when the subroutine does not find a library or version string for the function pointer attributes. Since these are optional it should not propagate the error up.

The current metadata parser for variable definitions fails to parse the following statement:

      type :: topflw_type
! total sky upward flux at toa
        real (kind=kind_phys) :: upfxc     
! clear sky upward flux at toa
        real (kind=kind_phys) :: upfx0    
      end type

It assumes that the type name is '::' rather than topflw_type.

The CCPP prebuild version is case-sensitive wrt scheme names in the suite definition files versus the Fortran file, i.e. if a Fortran file contains

module FoO
...
subroutine FoO_init(...)
...

then the SDF must contain

<scheme>FoO</scheme>

Since Fortran is case-insensitive, the module and subroutine names could use different capitalization. Therefore I believe that the matching of scheme names versus Fortran module/subroutine names should be case-insensitive.

Physics schemes and suites which contain internal state need a way to save and restore this state via host-model restart files. Following is a proposal for creating this functionality.
Two new optional CCPP scheme interfaces will be supported:

• <scheme>_restart_save(field_name, field_buffer): Given field_name, fill field_buffer with the appropriate data.
• <scheme>_restart_restore(field_name, field_buffer): Given field_name and field_buffer, restore the state of the appropriate internal buffer.

If these interfaces are present, they will be called from the host model cap via these new host cap interfaces:

• <host_name>_ccpp_physics_restart_save(suite_name, field_name, field_buffer): Call the appropriate <scheme>_restart_save routine.
• <host_name>_ccpp_physics_restart_restore(suite_name, field_name, field_buffer): Call the appropriate <scheme>_restart_restore routine.

In order for the host model to know which fields need to be written to and read from its restart files, any field which needs to be saved needs to set the (new) restart_required attribute to True. capgen will return a list of these fields and associated metadata to the host model build system.

It is an error for a physics scheme to have a field marked as restart_required = True and not provide <scheme>_restart_save and <scheme>_restart_restore interfaces.

The MacOSX build target was removed a while ago because of errors with the homebrew setup required to compile the code and run the tests. We need to update the homebrew config to make this work again. See comment in .travis.yml.

In order for the CCPP framework to function as a library that can be used with many physics libraries and suites, it should not require information for any suite or library to be in its repository.

The debugging output is enabled when ccpp-framework is compiled with -DCMAKE_BUILD_TYPE=Debug. The output generated can be extensive to overwhelming with large numbers of MPI tasks. We need a method to have the host model tell the framework for which tasks (none, a list of tasks or all) debugging output should be enabled.

All host model interface routines (except suite and suite part lists) must take a group name as an optional argument.

• At run time if no group is specified, run all groups in SDF order.
• At other times, only call routines associated with the specified group.

I propose an extension to the scheme syntax in the SDF. For example:

  <scheme execute_if=foo_execute_flag>foo</scheme>

Here foo is the scheme to run and foo_execute_flag is the standard name of a variable which is tested before a scheme is run.
Thoughts? @climbfuji? @davegill? @grantfirl? @llpcarson? @cacraigucar?

Users have reported that the actual error message coming from a physics scheme is not printed to screen, instead only "an error occurred in call to ccpp_physics_..." is returned. This may be an issue with the CCPP implementation in the respective model, or with the ccpp_prebuild framework itself.

How to test: in ufs-weather-model, use old ozone physics input file with new ozone physics activated or vice versa.

The following old metadata line is incorrectly converted when using convert_metadata.py using revision d447000

!! | theKinetics | kinetics_data | chemistry kinetics | DDT | 0 | kinetics_type | | none | F |

It becomes:
[theKinetics]
standard_name = kinetics_data
long_name = chemistry kinetics
units = DDT
dimensions = ()
type = kinetics_type
intent = none
optional = F

It should be:
[theKinetics]
standard_name = kinetics_data
long_name = chemistry kinetics
dimensions = ()
ddt_type = kinetics_type
intent = inout
optional = F

In the static build, files that are required for the dynamic build only are still generated:

ccpp_fields_{set}.inc
ccpp_modules_{set}.inc

where set is slow_physics and fast_physics in the case of FV3. This is confusing and should be removed.

When using v2.9.9 of libxml2, compilation of ccpp-framework failed:

Building C object ccpp/framework/src/CMakeFiles/ccpp.dir/ccpp_xml.c.o
In file included from /volumes/d1/grantf/gmtb-scm/ccpp/framework/src/ccpp_xml.c:31:
In file included from /opt/local/include/libxml2/libxml/parser.h:810:
In file included from /opt/local/include/libxml2/libxml/encoding.h:31:
/opt/local/include/unicode/ucnv.h:52:10: fatal error: 'unicode/ucnv_err.h' file not found
#include "unicode/ucnv_err.h"
^~~~~~~~~~~~~~~~~~~~
1 error generated.

Standard names should only contain letters, numbers and underscores. Right now, CCPP accepts and works with spaces in standard names (and possible other characters that shouldn't be allowed).

At line 399 in scripts/ccpp_suite.py, "pver" needs to be "pvar". Attempting to write out the error message results in the script core dumping.