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

Handle dimensions for promoted variables #562

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 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
40 changes: 40 additions & 0 deletions scripts/ccpp_suite.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from parse_tools import init_log, set_log_to_null
from suite_objects import CallList, Group, Scheme
from metavar import CCPP_LOOP_VAR_STDNAMES
from var_props import find_horizontal_dimension, find_vertical_dimension

# pylint: disable=too-many-lines

Expand Down Expand Up @@ -308,6 +309,45 @@ def find_variable(self, standard_name=None, source_var=None,
self.add_variable(var, self.__run_env)
# Remove the variable from the group
group.remove_variable(standard_name)
# Make sure the variable's dimensions are available
# at the init stage (for allocation)
for group in self.groups:
# only add dimension variables to init phase calling list
if group.name == self.__suite_init_group.name:
horiz_dim = find_horizontal_dimension(var.get_dimensions())[0]
vert_dim = find_vertical_dimension(var.get_dimensions())[0]
if horiz_dim and 'horizontal_loop' in horiz_dim:
# can't use horizontal_loop_being/end/extent in init phase
# must allocate to horizontal_dimension
new_horiz_dim = 'ccpp_constant_one:horizontal_dimension'
else:
new_horiz_dim = horiz_dim
# end if
if horiz_dim:
# make new variable with "correct" dimensions
if vert_dim:
new_dims = [new_horiz_dim, vert_dim]
else:
new_dims = [new_horiz_dim]
# end if
subst_dict = {'dimensions': new_dims}
prop_dict = var.copy_prop_dict(subst_dict=subst_dict)
temp_var = Var(prop_dict,
ParseSource(var.get_prop_value('scheme'),
var.get_prop_value('local_name'), var.context),
self.__run_env)
# Add dimensions if they're not already there
group.add_variable_dimensions(temp_var, [],
adjust_intent=True,
to_dict=group.call_list)
else:
# Add dimensions if they're not already there
group.add_variable_dimensions(var, [],
adjust_intent=True,
to_dict=group.call_list)
# end if
# end if
# end for
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is a case for this? Is it tested (capgen test seems to run fine without it)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The case would be a promoted variable with no horizontal dimension, but the existing dimension(s) do still need to be added to the calling list for the init routine.

That said, thanks to your other suggestion, I reworked the code so this "else" is no more. I also altered the capgen_test to ensure promotion/allocation works as expected for a variable with no horizontal dimension

else:
emsg = ("Group, {}, claimed it had created {} "
"but variable was not found")
Expand Down
17 changes: 15 additions & 2 deletions scripts/suite_objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -2230,9 +2230,22 @@ def analyze(self, phase, suite_vars, scheme_library, ddt_library,
self.run_env.logger.debug("{}".format(self))
# end if

def allocate_dim_str(self, dims, context):
def allocate_dim_str(self, dims, context, suite_var=False):
"""Create the dimension string for an allocate statement"""
rdims = list()
if suite_var:
# adjust horizontal dimension if horizontal_loop_* specified
horiz_dim = find_horizontal_dimension(dims)[0]
vert_dim = find_vertical_dimension(dims)[0]
if horiz_dim and 'horizontal_loop' in horiz_dim:
new_horiz_dim = 'ccpp_constant_one:horizontal_dimension'
if vert_dim:
dims = [new_horiz_dim, vert_dim]
else:
dims = [new_horiz_dim]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the only other possibility? No other dimensions allowed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A very good question.

Updated the logic in ccpp_suite to replace the horizontal dimension where it is and not remove other dimensions! This logic is now gone though, as I discovered it was unnecessary (the logic in ccpp_suite to promote variables handles the dimensions)

# end if
# end if
# end if
for dim in dims:
rdparts = list()
dparts = dim.split(':')
Expand Down Expand Up @@ -2500,7 +2513,7 @@ def write(self, outfile, host_arglist, indent, const_mod,
if dims:
timestep_var = svar.get_prop_value('persistence')
if group_type == timestep_var:
alloc_str = self.allocate_dim_str(dims, svar.context)
alloc_str = self.allocate_dim_str(dims, svar.context, suite_var=True)
lname = svar.get_prop_value('local_name')
outfile.write(alloc_stmt.format(lname, alloc_str),
indent+1)
Expand Down
3 changes: 2 additions & 1 deletion test/capgen_test/temp_adjust.F90
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,15 @@ MODULE temp_adjust
!! \htmlinclude arg_table_temp_adjust_run.html
!!
subroutine temp_adjust_run(foo, timestep, temp_prev, temp_layer, qv, ps, &
errmsg, errflg, innie, outie, optsie)
to_promote, errmsg, errflg, innie, outie, optsie)

integer, intent(in) :: foo
real(kind_phys), intent(in) :: timestep
real(kind_phys), intent(inout),optional :: qv(:)
real(kind_phys), intent(inout) :: ps(:)
REAL(kind_phys), intent(in) :: temp_prev(:)
REAL(kind_phys), intent(inout) :: temp_layer(foo)
real(kind_phys), intent(in) :: to_promote(:)
character(len=512), intent(out) :: errmsg
integer, optional, intent(out) :: errflg
real(kind_phys), optional, intent(in) :: innie
Expand Down
7 changes: 7 additions & 0 deletions test/capgen_test/temp_adjust.meta
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@
units = Pa
dimensions = (horizontal_loop_extent)
intent = inout
[ to_promote ]
standard_name = promote_this_variable_to_suite
units = K
dimensions = (horizontal_loop_extent)
type = real
kind = kind_phys
intent = in
[ errmsg ]
standard_name = ccpp_error_message
long_name = Error message for error handling in CCPP
Expand Down
3 changes: 2 additions & 1 deletion test/capgen_test/temp_set.F90
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ MODULE temp_set
!! \htmlinclude arg_table_temp_set_run.html
!!
SUBROUTINE temp_set_run(ncol, lev, timestep, temp_level, temp, ps, &
errmsg, errflg)
to_promote, errmsg, errflg)
!----------------------------------------------------------------
IMPLICIT NONE
!----------------------------------------------------------------
Expand All @@ -29,6 +29,7 @@ SUBROUTINE temp_set_run(ncol, lev, timestep, temp_level, temp, ps, &
real(kind_phys), intent(in) :: timestep
real(kind_phys), intent(in) :: ps(:)
REAL(kind_phys), INTENT(inout) :: temp_level(:, :)
real(kind_phys), intent(out) :: to_promote(:, :)
character(len=512), intent(out) :: errmsg
integer, intent(out) :: errflg
!----------------------------------------------------------------
Expand Down
7 changes: 7 additions & 0 deletions test/capgen_test/temp_set.meta
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@
units = Pa
dimensions = (horizontal_loop_extent)
intent = in
[ to_promote ]
standard_name = promote_this_variable_to_suite
units = K
dimensions = (horizontal_loop_extent, vertical_layer_dimension)
type = real
kind = kind_phys
intent = out
[ errmsg ]
standard_name = ccpp_error_message
long_name = Error message for error handling in CCPP
Expand Down
Loading