gold2718 / ccpp-framework Goto Github PK

The CPF needs to pass in one or more available unit numbers for a scheme to use for its reading of namelists and static data. It also needs to pass in the filename(s)

Perhaps the unit numbers become a specially identified variable in the metadata. If more than one is requested, then they each need to be unique.

I get the following errors:

/home/cacraig/CPF_proto_new_cam_kessler/cam_driver/bin/ccpp/ccpp_cam_kessler_test_cap.F90:252:44:

   call kessler_timestep_finalize(col_end - col_start + 1, pver,            &
                                        1

Error: Symbol ‘col_end’ at (1) has no IMPLICIT type
/home/cacraig/CPF_proto_new_cam_kessler/cam_driver/bin/ccpp/ccpp_cam_kessler_test_cap.F90:252:56:

   call kessler_timestep_finalize(col_end - col_start + 1, pver,            &
                                                    1

Error: Symbol ‘col_start’ at (1) has no IMPLICIT type

This is the first time I'm trying to pass ncol to timestep_init and timestep_finalize. The error occurs with both routines.

Right now, the order of variables between the .F90 and .meta files must be an exact match or an error occurs during the capgen step. Since the local names must match in both files, it should be possible to link them even if the list of variables are in different order

Using a kind of "rk" works through the capgen, but fails during the make step. The file ccpp_MusicBox_suite_cap.F90 contains declarations using rk, but does not have a "use" statement to bring it in.

To demonstrate the issue, use MusicBox_cpf_0013 and change all instances of kind_phys to rk inside MICM_chemistry/src/tuv/photo_rates/tuv_photolysis (.F90 and .meta) .

The error is as follows:

[ 76%] Building Fortran object CMakeFiles/MusicBox.dir/ccpp/ccpp_MusicBox_suite_cap.F90.o
/home/cacraig/MusicBox_splitTUV/MusicBox_host/build/ccpp/ccpp_MusicBox_suite_cap.F90:154:13:
real(rk), intent(out) :: tuv_prates(:,:)
1
Error: Symbol ‘rk’ at (1) has no IMPLICIT type
/home/cacraig/MusicBox_splitTUV/MusicBox_host/build/ccpp/ccpp_MusicBox_suite_cap.F90:185:22:

        tuv_prates(:, :), errmsg, errflg)
                 1

-Error: Syntax error in argument list at (1)
/home/cacraig/MusicBox_splitTUV/MusicBox_host/build/ccpp/ccpp_MusicBox_suite_cap.F90:186:50:

   call photolysis_interstitial_run(tuv_prates(:, :), j_rateConst, errmsg, errflg)
                                              1

Error: Syntax error in argument list at (1)
/home/cacraig/MusicBox_splitTUV/MusicBox_host/build/ccpp/ccpp_MusicBox_suite_cap.F90:120:41:

     o3vmr, so2vmr, no2vmr, tuv_prates, TimeStart, TimeEnd)
                                     1

Error: Symbol ‘tuv_prates’ at (1) has no IMPLICIT type
make[2]: *** [CMakeFiles/MusicBox.dir/ccpp/ccpp_MusicBox_suite_cap.F90.o] Error 1
make[1]: *** [CMakeFiles/MusicBox.dir/all] Error 2
make: *** [all] Error 2

As the generated code uses variable names from underlying routines (which may have differing names), it would be useful to have standard names appended to the variable declarations as comments. This will be useful for anyone examining the code and also for users who are copying sections to go back into non-ccpp code as they will need to translate these variables.

When two schemes reference the same variable and the intent of the first scheme is "in" and the second scheme is "inout", the code for the generated MusicBox_suite_physics is mistakenly having the variable declared as "in". This errors out during the make step.

To demonstrate this, clone MusicBox_cpf_0013 and checkout "cpf". Then modify the intent for mass_quantities_util (.F90 and .meta) so that is intent "in" instead of "inout". Intent "in" is what this routine should have the variable set as. "inout" is my temporary workaround.

Either I didn't completely implement this correctly, or there is a bug in the framework. When I moved precl as we discussed, I get the following error during compilation:

/home/cacraig/CPF_proto_new_cam_kessler/cam_driver/bin/ccpp/CAM_ccpp_cap.F90:33:7:

use cam_kessler_main, only: precl
   1

Fatal Error: Can't open module file ‘cam_kessler_main.mod’ for reading at (1): No such file or directory
compilation terminated.

It looks like cam_kessler_main isn't compiled yet. Is this another instance like "machines"?

There is a "comma" in the private suite declarations when there are no specifiers for both integer and character variables.

/home/cacraig/CPF_proto_kessler_micm/cam_driver/bin/ccpp/ccpp_cam_kessler_micm_cap.F90:36:11:

integer,                          :: nSpecies
       1

Error: Invalid character in name at (1)

These errors are in addition to the previously reported missing type. NOTE - the type declaration will also need the use statement to bring it in.

/home/cacraig/CPF_proto_kessler_micm/cam_driver/bin/ccpp/ccpp_cam_kessler_micm_cap.F90:37:9:

type(),             allocatable   :: cnst_info(:)
     1

Error: Invalid character in name at (1)

if a scheme has an intent(out) variable, the framework automatically allocates that variable. However, if the scheme intends to do the allocation, the framework should not insert a duplicate allocation call.
The requirements for the framework not allocating the variable are:

• allocatable = True is in the variable's metadata
• The variable is declared with the allocatable or pointer attribute in the associated Fortran code.
• The variables which determine the size of the array must also be intent(out) arguments of the scheme.

All of the other substitution strings are one word (with underscore separating individual words). 'enter units' is two words. As a convenience for editors like vi this too should be one word

Currently the host model developer needs to augment their call with the host model data to match the call that is being auto-generated for CAM_ccpp_physics_initialize. This is not ideal for the developer as there is no way to know what the order is until a cmake is completed.

When using the CPF_0.2.021 tag, which is the same tag that is currently used in CAMDEN, the "HelloWorld" example README.md document contains a reference to a --generate-host-cap argument for ccpp_capgen.py which no longer appears to exist, and which will cause that example to fail. This missing argument will also cause building with the Makefile.example file to fail as well. However, if the argument is left out then ccpp_capgen.py appears to work without any problems.

I am not sure if this argument exists for other ccpp-framework versions, but if this particular branch/fork is the one that will be moving forward then I would recommend removing the --generate-host-cap argument from "HelloWorld", just to avoid any confusion for new users.

Routines with names such as <scheme>_init and <scheme>_timestep_init are supposed to be automatically assigned to the correct group, however, <scheme>_timestep_init is matching the <scheme>_init regular expression and sending those routines to the wrong group.

While attempting to run the registry generator in CAMDEN with python 3.7, I ran into the following error:

File "/home/runner/work/repoForTestingCAM/repoForTestingCAM/src/data/generate_registry_data.py", line 1502, in gen_registry
    error_on_noxmllint=error_on_no_validate)
  File "/home/runner/work/repoForTestingCAM/repoForTestingCAM/ccpp_framework/scripts/parse_tools/xml_tools.py", line 200, in validate_xml_file
    result = call_command(cmd, logger)
  File "/home/runner/work/repoForTestingCAM/repoForTestingCAM/ccpp_framework/scripts/parse_tools/xml_tools.py", line 53, in call_command
    stderr=subprocess.STDOUT)
  File "/opt/hostedtoolcache/Python/3.7.6/x64/lib/python3.7/subprocess.py", line 483, in run
    raise ValueError('stdout and stderr arguments may not be used '
ValueError: stdout and stderr arguments may not be used with capture_output.

After tracing it through, I believe it is due to this specific line of code in xml_tools.py (Line 51-53):

cproc = subprocess.run(commands, check=True,
                                       capture_output=True,
                                       stderr=subprocess.STDOUT)

which might no longer be allowed in Python version 3.7 or later, based off this text in the official 3.7 documentation:

If capture_output is true, stdout and stderr will be captured. When used, the internal Popen object is automatically created with stdout=PIPE and stderr=PIPE. The stdout and stderr arguments may not be supplied at the same time as capture_output. If you wish to capture and combine both streams into one, use stdout=PIPE and stderr=STDOUT instead of capture_output.

The version of the code I am using to generate this error is tag CPF_0.2.020.

The generated ccpp_kessler_cam_cap.F90 has OMP_GET_THREAD_NUM as a variable inside "#ifdef _OPENMP" sections. When _OPENMP evaluates to true, there is not declaration for this variable and compiler errors are raised.

Currently the CCPP capgen datatable XML file is printed on a single line, which makes it difficult to read in the rare instances when a person might want to directly examine the file.

Thus it would be nice if this file was "pretty-printed" such that it is easier to read directly. Hopefully this would have no impact on the CCPP framework's use of the datatable file itself.

In order to handle a polymorphic array of DDTs, the class declaration must be used instead of type. This could be triggered by introducing a new logical metadata keyword, polymorphic.

Having a ddt followed by a scheme gives the following error. Until I get my program running correctly, I won't definitively say this is a bug. After I get kessler running with the updated CPF, I can try putting the ddt and scheme back together in one file and see if I get the error. For now this is placeholder issue in case it is truly a bug.

-- Disable OpenMP support for C/C++/Fortran compiler
-- HOST cam7_kessler ABS_PATH /home/cacraig/CPF_proto_kessler01_000/cam_driver/src/state_converters.F90
-- HOST cam7_kessler ABS_PATH /home/cacraig/CPF_proto_kessler01_000/cam_driver/src/kessler.F90
-- HOST cam7_kessler ABS_PATH /home/cacraig/CPF_proto_kessler01_000/cam_driver/src/kessler_update.F90
-- Running: /home/cacraig/CPF_proto_kessler01_000/cam_driver/../ccpp-framework/scripts/ccpp_capgen.py --host-files ../src/physics_types_cam7.meta,../src/cam7_kessler.meta --scheme-files ../../simple/src/simple_schemes.txt --suites ../suites/suite_cam_kessler_test_simple1.xml --output-root /home/cacraig/CPF_proto_kessler01_000/cam_driver/src/ccpp --generate-host-cap --host-name cam7_kessler --verbose
-- Reading host model data from /home/cacraig/CPF_proto_kessler01_000/cam_driver/src/physics_types_cam7.meta
Metadata table type mismatch for physics_tend, scheme != ddt, at physics_types_cam7.meta:316

While there are datatable reports for different variable intents, there is no report for a variable which is intent(in) or intent(inout) at run time but which is intent(out) at initialize time. This feature could be used to select only the variables which need to be initialized.

@gold2718 @AndrewJConley

From the email chain prior to opening this issue:

Yes, this helps. Now, for nesting, would you expect it to work as if a <suite ...> entry in a suite works like an include statement? That is, it would be as if you cut and pasted the contents of the suite into that location. If not, please explain how you think this should work.

One important requirement is that the proposed <suite ...> entry must include both a suite name and a group name so that the schemes from the correct run-time group can be 'copied' into place. However, it is less clear on how to handle initialization. Do we have to initialize the entire nested suite? If so, where does it go in the initialization sequence? If not, how do we know when initialization (and finalization) routines to run and when to run them?

All of this needs to be discussed in the GitHub issue.

--Steve

On Fri, Aug 16, 2019 at 12:14 PM Andrew Conley [email protected] wrote:
Hi Steve,

It is not a loop over species, but a loop over reactions. Every reaction has a rate constant associated with that rate constant. Each rate constant depends upon a heterogenous set of environmental factors (temperature, pressure, surface area density, mean radius of the cloud droplet, or some other random thing). The units of the returned rate constant is also distinct between each rate constant computation.

So each rate constant seems to be ideally qualified to be its own scheme.

The nesting is not terribly relevant, but the idea of separate schemes seems appropriate to the CPF framework standards.

Does that help?

-- Andrew

I discovered that a scheme which is the second one called using the array o2_xsect had incorrect names for the dimension names in the .meta file. There is no indication from the capgen routine that these dimension names are incorrect (not even valid names) and do not match the dimension names from the first scheme.

The files to demonstrate this bug are MICM_chemistry/src/tuv/oxygen_xsect/molec_ox_xsect.meta and MICM_chemistry/src/tuv/photo_rates/tuv_photolysis.meta. This bug has existed in MICM_chemistry for at least the last month or so, so any recent version of MusicBox should demonstrate the problem.

If a DDT is intent(out) and it contains an allocatable array, the array should be allocated by the CPF prior to the call.

I am getting an error message:
check intent

There is no other information (which variable, which routine, etc.)

As the CPF interface expands to include 5 types of calls for each scheme, it will become more important to short circuit calling empty routines. In the case of chemistry, this could eliminate hundreds of calls to empty routines on each time step.

I would propose that the requirement be that every metadata file lists the 5 types of scheme calls. I would also propose a keyword at the ccpp-arg-table section of the metadata for a named scheme's subroutine which is something like: "empty_scheme = true". This would then trigger the CPF to pop this particular subroutine off of its stack of calls to generate.

This has the added benefit that individual scheme .F90 files would only need to include the subroutine types which actually are needed and not need to write dummy routines for the others.

This list needs to include "flux"

There needs to be a metadata attribute that defines a new, user-level dimension which is then accepted by the framework as a valid dimension.

When running kessler using NAG in non-debug mode, get "Error: cam_ccpp_physics_finalize". Inspecting the code it looks like the generated code for kessler_cam_finalize in ccpp_kessler_cam_cap needs to have errmsg and errflg changed to intent(inout) from intent(out).

I'm unable to test this theory as when I modify the code and do a case.build, it regenerates the cap.

Error checking between invocation of scheme subroutines is missing in the suite cap code. An exception was thrown by a scheme init (and run) without being caught. The errflg was set to non-zero in the first scheme then reset to zero in the next scheme. An exception needs to be thrown in ccpp_MusicBox_suite_cap code without moving on the the next scheme in the suite.

While creating a python unit test for CAMDEN with this registry file:

<?xml version="1.0" encoding="UTF-8"?>

<registry name="cam_registry" version="1.0">
  <file name="physics_types_ddt2" type="module">
    <use module="ccpp_kinds" reference="kind_phys"/>
    <variable local_name="theta" standard_name="potential_temperature"
              units="K" type="real" kind="kind_phys"  allocatable="pointer">
    </variable>
    <ddt type="model_thermo">
      <data>potential_temperature</data>
    </ddt>
    <variable local_name="thermo" standard_name="cam_thermo"
              units="None" type="model_thermo">
    </variable>
    <ddt type="physics_state">
      <data>cam_thermo</data>
    </ddt>
    <variable local_name="phys_state"
              standard_name="physics_state_from_dynamics"
              units="None" type="physics_state">
    </variable>
  </file>
</registry>

generate_registry.py fails with this traceback:

Traceback (most recent call last):
File "write_init_unit_tests.py", line 510, in test_ddt2_reg_write_init
error_on_no_validate=True)
File "/glade/work/nusbaume/SE_projects/new_cam_sandbox/CAMDEN/src/data/generate_registry_data.py", line 1465, in gen_registry
files = write_registry_files(registry, dycore, config, outdir, indent, logger)
File "/glade/work/nusbaume/SE_projects/new_cam_sandbox/CAMDEN/src/data/generate_registry_data.py", line 1389, in write_registry_files
file_.write_source(outdir, indent, logger)
File "/glade/work/nusbaume/SE_projects/new_cam_sandbox/CAMDEN/src/data/generate_registry_data.py", line 1170, in write_source
self.write_allocate_routine(outfile)
File "/glade/work/nusbaume/SE_projects/new_cam_sandbox/CAMDEN/src/data/generate_registry_data.py", line 1226, in write_allocate_routine
var.write_allocate_routine(outfile, 2, init_var, reall_var, '')
File "/glade/work/nusbaume/SE_projects/new_cam_sandbox/CAMDEN/src/data/generate_registry_data.py", line 647, in write_allocate_routine
init_var, reall_var, sub_ddt_str)
File "/glade/work/nusbaume/SE_projects/new_cam_sandbox/CAMDEN/src/data/generate_registry_data.py", line 647, in write_allocate_routine
init_var, reall_var, sub_ddt_str)
File "/glade/work/nusbaume/SE_projects/new_cam_sandbox/CAMDEN/src/data/generate_registry_data.py", line 669, in write_allocate_routine
outfile.write("call endrun({})".format(emsg), indent+2)
File "/glade/work/nusbaume/SE_projects/new_cam_sandbox/CAMDEN/ccpp_framework/scripts/fortran_tools/fortran_write.py", line 119, in write
self.write(statement, indent_level, continue_line=line_continue)
File "/glade/work/nusbaume/SE_projects/new_cam_sandbox/CAMDEN/ccpp_framework/scripts/fortran_tools/fortran_write.py", line 112, in write
if line_continue:
UnboundLocalError: local variable 'line_continue' referenced before assignment

This occurs with both python 3.6.8 and python 2.7.13.

Thus it appears that a situation arises where line_continue is called without being set. I am not 100% sure yet if this is a CAMDEN issue, a CCPP framework issue, or I simply have a bad registry file. However, given that the actual error appears in the CCPP framework, and the outfile call in CAMDEN doesn't appear to be wrong, I decided to add the issue here. I'll hopefully be able to narrow down on the specific cause of the problem later, and will post the results here.

It would be helpful (if it is easy to do) to include the incorrect name for the index variable.

The current error message is:
Parsing metadata_file, '/home/cacraig/CAMDEN_heldsuarez/src/dynamics/utils/vert_coord.meta'
Parsing file, physics_types, from registry
Traceback (most recent call last):
File "/home/cacraig/CAMDEN_heldsuarez/cime_config/buildnml", line 293, in
_main_func()
File "/home/cacraig/CAMDEN_heldsuarez/cime_config/buildnml", line 289, in _main_func
buildnml(case, caseroot, "cam")
File "/home/cacraig/CAMDEN_heldsuarez/cime_config/buildnml", line 127, in buildnml
config.generate_cam_src(gen_indent)
File "/home/cacraig/CAMDEN_heldsuarez/cime_config/cam_config.py", line 1022, in generate_cam_src
dyn, gen_fort_indent)
File "/home/cacraig/CAMDEN_heldsuarez/cime_config/cam_autogen.py", line 323, in generate_registry
error_on_no_validate=True)
File "/home/cacraig/CAMDEN_heldsuarez/src/data/generate_registry_data.py", line 1544, in gen_registry
src_root, reg_dir, indent, logger)
File "/home/cacraig/CAMDEN_heldsuarez/src/data/generate_registry_data.py", line 1421, in write_registry_files
files.append(File(section, known_types, dycore, config, logger))
File "/home/cacraig/CAMDEN_heldsuarez/src/data/generate_registry_data.py", line 1061, in init
self.add_variable(obj, logger)
File "/home/cacraig/CAMDEN_heldsuarez/src/data/generate_registry_data.py", line 1086, in add_variable
logger)
File "/home/cacraig/CAMDEN_heldsuarez/src/data/generate_registry_data.py", line 529, in init
vdict))
File "/home/cacraig/CAMDEN_heldsuarez/src/data/generate_registry_data.py", line 362, in init
raise CCPPError(emsg.format(self.index_name, parent_name))
File "/home/cacraig/CAMDEN_heldsuarez/src/data/generate_registry_data.py", line 313, in index_name
return self.__index_name
AttributeError: 'ArrayElement' object has no attribute '_VarBase__index_name'
ERROR: /home/cacraig/CAMDEN_heldsuarez/cime_config/buildnml /home/cacraig/CAMDEN_heldsuarez/cime/scripts/test_camden_kessler_try06 FAILED, see above

This issue has bitten me again and probably will be fixed in this repo (and may get lost in the NCAR repo issues) which is why I'm duplicating it here. I was doing code cleanup, and removed a dimension which was in the calling list of a routine, but not being used in it internally. It turns out that this variable was the one I put in to workaround this issue.

The standard name of the variable in question is "variable_layer_dimension". This is assigned a value in the host model and resides as a host level module data field and is known as nlayer. When I have a array which does not exist at the host model level, but only at the scheme level the allocate for the array inside ccpp_MusicBox_suite_cap.F90 says
allocate(dto2(1:nlayer,1:tuv_n_wavelen)). The error is that "symbol nlayer has no IMPLICIT type"

[NCAR/ccpp-framework] Require variable dimensions in argument list? (NCAR#197)

When the mandatory attribute "intent" is missing the following is reported. It doesn't give a hint of what is wrong.

-- Traceback (most recent call last):
File "/home/cacraig/CPF_proto_new_cam_kessler/ccpp-framework/scripts/ccpp_capgen.py", line 271, in
_main_func()
File "/home/cacraig/CPF_proto_new_cam_kessler/ccpp-framework/scripts/ccpp_capgen.py", line 247, in _main_func
ccpp_api = API(sdfs, host_model, scheme_headers, logger)
File "/home/cacraig/CPF_proto_new_cam_kessler/ccpp-framework/scripts/ccpp_suite.py", line 896, in init
suite.analyze(host_model, scheme_headers, logger)
File "/home/cacraig/CPF_proto_new_cam_kessler/ccpp-framework/scripts/ccpp_suite.py", line 767, in analyze
item.analyze(phase, self, scheme_headers, logger)
File "/home/cacraig/CPF_proto_new_cam_kessler/ccpp-framework/scripts/ccpp_suite.py", line 431, in analyze
lschemes, lvars = item.analyze(phase, self, scheme_headers, suite_vars, logger)
File "/home/cacraig/CPF_proto_new_cam_kessler/ccpp-framework/scripts/ccpp_suite.py", line 240, in analyze
intent = svar.get_prop_value('intent').lower()
AttributeError: 'NoneType' object has no attribute 'lower'

CMake Error at CMakeLists.txt:140 (MESSAGE):
CCPP cap generation FAILED: result = 1

In a ddt declaration, when OTHER fields were missing in the .meta file, I received the following misleading error message:

Variable mismatch in physics_state, no Fortran variable q(:, :, index_of_water_vapor_specific_humidity).
Variable mismatch in physics_state, no Fortran variable q(:, :, index_of_cloud_liquid_water_mixing_ratio).
Variable mismatch in physics_state, no Fortran variable q(:, :, index_of_rain_water_mixing_ratio).
3 errors found comparing /home/cacraig/CPF_proto_kessler01_000/cam_driver/src/physics_ddt_cam7.meta to /home/cacraig/CPF_proto_kessler01_000/cam_driver/src/physics_ddt_cam7.F90

To duplicate this remove any field from the test_host_data.meta file (u for example), and you will get a message about the q(:,:,index_of_water_vapor_specific_humidity)

I'm classifying it as a bug, because I sunk way too many hours into trying to fix the index_of_xxxx problem when it wasn't the issue at all. It will cause others to go down the same path.

I believe a better name for the "allocatable" attribute is "allocated".

"allocatable" means that it is array that will be allocated

"allocated" means that the array has been allocated

I believe "allocated" better describes the state of the array when it is being passed out of a scheme which has internally allocated it.

When two different routines use the same local name for variables which are matched to different standard names, the local name is entered twice into the ccpp_MusicBox_suite_cap.F90, which causes a compilation error.

To demonstrate this, clone cacraigucar/MusicBox and checkout hash e045a28732553141849c1fbdcd74377386070160. Inside MusicBox_host/src/relhum/relhum_mod.F90 and .meta, remove "box" from in front of boxtemp, boxpress, boxh2ovmr, and boxrelhum in all locations. "temp" will then collide with another module which is using a different local "temp" variable.

I receive an error "Unknown DDT type, const_props_type, at /home/cacraig/MusicBox_bug/MusicBox_host/src/calc_molar_mass.meta:8" when the file with the DDT definition is listed after the calc_molar_mass file in my file list.

To demonstrate this,checkout NCAR/MusicBox 667e9a5bae745c7860a03dc100d94615be562c30

In MusicBox_host/src/MusicBox_scheme_files.txt, move the const_props_mod.meta line to the bottom of the file

This could also be a bug in my CMakeLists.txt file (maybe DDTs belong in a different file list?)