Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 18 additions & 15 deletions scripts/metavar.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ class Var:
# All constituent props are optional so no check

def __init__(self, prop_dict, source, run_env, context=None,
clone_source=None):
clone_source=None, skip_checks=False):
"""Initialize a new Var object.
If <prop_dict> is really a Var object, use that object's prop_dict.
If this Var object is a clone, record the original Var object
Expand Down Expand Up @@ -361,18 +361,20 @@ def __init__(self, prop_dict, source, run_env, context=None,
# # end for
# XXgoldyXX: ^ don't fill in default properties?
# Make sure all the variable values are valid
try:
for prop_name, prop_val in self.var_properties():
prop = Var.get_prop(prop_name)
_ = prop.valid_value(prop_val,
prop_dict=self._prop_dict, error=True)
# end for
except CCPPError as cperr:
lname = self._prop_dict['local_name']
emsg = "{}: {}"
raise ParseSyntaxError(emsg.format(lname, cperr),
context=self.context) from cperr
# end try
if not skip_checks:
try:
for prop_name, prop_val in self.var_properties():
prop = Var.get_prop(prop_name)
_ = prop.valid_value(prop_val,
prop_dict=self._prop_dict, error=True)
# end for
except CCPPError as cperr:
lname = self._prop_dict['local_name']
emsg = "{}: {}"
raise ParseSyntaxError(emsg.format(lname, cperr),
context=self.context) from cperr
# end try
# end if

def compatible(self, other, run_env, is_tend=False):
"""Return a VarCompatObj object which describes the equivalence,
Expand Down Expand Up @@ -1209,9 +1211,10 @@ def __init__(self, prop_dict, source, run_env, context=None,
del prop_dict[prop.name]
# end if
# end for
# Initialize Var
# Initialize Var; skip the parse checkers on the Fortran side since the
# checks are already done during metadata parsing
super().__init__(prop_dict, source, run_env, context=context,
clone_source=clone_source)
clone_source=clone_source, skip_checks=True)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like the wrong place to add this check as the problem (which I do not really follow without a regression test to read) is not within this type but its use in parse_fortran.py.
I suggest adding skip_checks as an input to the __init__ method for this class and calling it from parse_fortran.py

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added some new test files to the ddthost test that will cause the test to fail without the changes in this PR. Hope that helps - and thanks for the suggestion to add a test.

I'm not sure I'm following your suggestion here. The checks are done in super().__init__ so I don't know how to avoid passing the skip_checks into there. I figured since the FortranVar class is only used during fortran parsing, it would be unnecessary to add the flag to the FortranVar.__init__. But if I'm missing something please let me know!

# Now, restore the saved properties
for prop in save_dict:
self._prop_dict[prop] = save_dict[prop]
Expand Down
8 changes: 7 additions & 1 deletion test/ddthost_test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
#------------------------------------------------------------------------------
set(SCHEME_FILES "setup_coeffs" "temp_set" "temp_adjust" "temp_calc_adjust")
set(SUITE_SCHEME_FILES "make_ddt" "environ_conditions")
set(HOST_FILES "test_host_data" "test_host_mod" "host_ccpp_ddt")
set(HOST_FILES "test_host_data" "test_host_mod" "host_ccpp_ddt" "wrapped_ddt")
set(SUITE_FILES "ddt_suite.xml" "temp_suite.xml")
# HOST is the name of the executable we will build.
# We assume there are files ${HOST}.meta and ${HOST}.F90 in CMAKE_SOURCE_DIR
Expand Down Expand Up @@ -39,12 +39,18 @@ ccpp_capgen(CAPGEN_DEBUG ON
ccpp_datafile(DATATABLE "${CCPP_CAP_FILES}/datatable.xml"
REPORT_NAME "--ccpp-files")

# Create dependent library to mimic possibly pre-built external dependencies
add_library(external_module STATIC external_module.F90)

# Create test host library
add_library(DDT_TESTLIB OBJECT ${SCHEME_FORTRAN_FILES}
${SUITE_SCHEME_FORTRAN_FILES}
${DDT_HOST_FORTRAN_FILES}
${CCPP_CAPS_LIST})

# Add the external as a dependency of the DDT library.
add_dependencies(DDT_TESTLIB external_module)

# Setup test executable with needed dependencies
add_executable(ddt_host_integration test_ddt_host_integration.F90 ${HOST}.F90)
target_link_libraries(ddt_host_integration PRIVATE DDT_TESTLIB test_utils)
Expand Down
2 changes: 2 additions & 0 deletions test/ddthost_test/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
Contains tests to exercise more DDT functionality:
- Passing around and modifying a DDT
- Making DDT in host model & using it in CCPP-ized physics code
- Providiing metadata for only the top-level DDT
- Ensures framework allows unknown DDT in Fortran as long as there's no metadata for it

## Building/Running

Expand Down
7 changes: 7 additions & 0 deletions test/ddthost_test/external_module.F90
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module external_module

type, public :: unknown_external_ddt
integer :: var
end type

end module external_module
6 changes: 5 additions & 1 deletion test/ddthost_test/temp_adjust.F90
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ module temp_adjust
!> \section arg_table_temp_adjust_run Argument Table
!! \htmlinclude arg_table_temp_adjust_run.html
!!
subroutine temp_adjust_run(foo, timestep, temp_prev, temp_layer, qv, ps, &
subroutine temp_adjust_run(foo, timestep, wrapped_ext_ddt, temp_prev, temp_layer, qv, ps, &
to_promote, promote_pcnst, errmsg, errflg, innie, outie, optsie)
use wrapped_ddt, only: wrapped_ddt_t

integer, intent(in) :: foo
real(kind_phys), intent(in) :: timestep
type(wrapped_ddt_t), intent(out) :: wrapped_ext_ddt
real(kind_phys), intent(inout),optional :: qv(:)
real(kind_phys), intent(inout) :: ps(:)
real(kind_phys), intent(in) :: temp_prev(:)
Expand All @@ -40,6 +42,8 @@ subroutine temp_adjust_run(foo, timestep, temp_prev, temp_layer, qv, ps, &
errmsg = ''
errflg = 0

wrapped_ext_ddt%ext_ddt%var = 1

do col_index = 1, foo
temp_layer(col_index) = temp_layer(col_index) + temp_prev(col_index)
if (present(qv)) qv(col_index) = qv(col_index) + 1.0_kind_phys
Expand Down
6 changes: 6 additions & 0 deletions test/ddthost_test/temp_adjust.meta
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@
type = real
kind = kind_phys
intent = in
[ wrapped_ext_ddt ]
standard_name = wrapped_ddt_for_physics
units = none
dimensions = ()
type = wrapped_ddt_t
intent = out
[ temp_prev ]
standard_name = potential_temperature_at_previous_timestep
units = K
Expand Down
11 changes: 11 additions & 0 deletions test/ddthost_test/wrapped_ddt.F90
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
module wrapped_ddt
! CCPP wrapper for external ddt
use external_module, only: unknown_external_ddt

!> \section arg_table_wrapped_ddt_t Argument Table
!! \htmlinclude wrapped_ddt_t.html
type, public :: wrapped_ddt_t
type(unknown_external_ddt) :: ext_ddt
end type

end module wrapped_ddt
7 changes: 7 additions & 0 deletions test/ddthost_test/wrapped_ddt.meta
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[ccpp-table-properties]
name = wrapped_ddt_t
type = ddt

[ccpp-arg-table]
name = wrapped_ddt_t
type = ddt