Skip to content

Commit

Permalink
#2244 Fix more issues processing LFRic trunk (it now compiles and run…
Browse files Browse the repository at this point in the history
… gungho, but threading is in the wrong loop)
  • Loading branch information
sergisiso committed Oct 22, 2024
1 parent bd96a85 commit 1752893
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 44 deletions.
2 changes: 2 additions & 0 deletions src/psyclone/domain/lfric/lfric_constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ def __init__(self):
# coloured loop
"ntilecolours", # the number of colours in a
# coloured tiled loop
"last_edge_tile_per_colour",
"last_edge_cell_per_coloured_tile",
"ncells", # the number of owned cells
"ndofs", # the number of owned dofs
"nannexed"] # the number of owned dofs
Expand Down
12 changes: 12 additions & 0 deletions src/psyclone/domain/lfric/lfric_loop.py
Original file line number Diff line number Diff line change
Expand Up @@ -468,10 +468,22 @@ def _upper_bound_fortran(self):
if Config.get().distributed_memory:
return (f"{self._mesh_name}%get_last_halo_tile_per_colour("

Check warning on line 469 in src/psyclone/domain/lfric/lfric_loop.py

View check run for this annotation

Codecov / codecov/patch

src/psyclone/domain/lfric/lfric_loop.py#L468-L469

Added lines #L468 - L469 were not covered by tests
f"{halo_index})")
raise GenerationError(

Check warning on line 471 in src/psyclone/domain/lfric/lfric_loop.py

View check run for this annotation

Codecov / codecov/patch

src/psyclone/domain/lfric/lfric_loop.py#L471

Added line #L471 was not covered by tests
"'last_halo_tile_per_colour' is not a valid loop upper bound "
"for non-distributed-memory code")
if self._upper_bound_name == "last_halo_cell_per_colour_and_tile":
if Config.get().distributed_memory:
return (f"{self._mesh_name}%get_last_halo_cell_per_colour_and"

Check warning on line 476 in src/psyclone/domain/lfric/lfric_loop.py

View check run for this annotation

Codecov / codecov/patch

src/psyclone/domain/lfric/lfric_loop.py#L475-L476

Added lines #L475 - L476 were not covered by tests
f"_tile({halo_index})")
raise GenerationError(

Check warning on line 478 in src/psyclone/domain/lfric/lfric_loop.py

View check run for this annotation

Codecov / codecov/patch

src/psyclone/domain/lfric/lfric_loop.py#L478

Added line #L478 was not covered by tests
"'last_halo_cell_per_colour_and_tile' is not a valid loop "
"upper bound for non-distributed-memory code")
if self._upper_bound_name == "last_edge_tile_per_colour":
raise GenerationError(

Check warning on line 482 in src/psyclone/domain/lfric/lfric_loop.py

View check run for this annotation

Codecov / codecov/patch

src/psyclone/domain/lfric/lfric_loop.py#L482

Added line #L482 was not covered by tests
"FIXME: 'last_egde_tile_per_colour'")
if self._upper_bound_name == "last_edge_tile_per_colour":
raise GenerationError(

Check warning on line 485 in src/psyclone/domain/lfric/lfric_loop.py

View check run for this annotation

Codecov / codecov/patch

src/psyclone/domain/lfric/lfric_loop.py#L485

Added line #L485 was not covered by tests
"FIXME: 'last_edge_cell_per_coloured_tile")

if self._upper_bound_name == "ncolour":
# Loop over cells of a particular colour when DM is disabled.
Expand Down
86 changes: 50 additions & 36 deletions src/psyclone/dynamo0p3.py
Original file line number Diff line number Diff line change
Expand Up @@ -764,8 +764,8 @@ def initialise(self, parent):
# it now, rather than when this class was first constructed.
need_colour_limits = False
need_colour_halo_limits = False
need_tilecolour_limits = False
need_tilecolour_halo_limits = False
# need_tilecolour_limits = False
# need_tilecolour_halo_limits = False
for call in self._calls:
if call.is_coloured() and not call.is_intergrid:

Expand All @@ -774,12 +774,14 @@ def initialise(self, parent):
const.HALO_ACCESS_LOOP_BOUNDS)
if has_halo:
if is_tiled:
need_tilecolour_halo_limits = True
# need_tilecolour_halo_limits = True
pass
else:
need_colour_halo_limits = True
else:
if is_tiled:
need_tilecolour_limits = True
# need_tilecolour_limits = True
pass
else:
need_colour_limits = True

Expand Down Expand Up @@ -849,6 +851,7 @@ def initialise(self, parent):
# rhs = f"{mesh}%get_last_edge_cell_per_colour_and_tile()"
# parent.add(AssignGen(parent, lhs=lhs, rhs=rhs))


class DynReferenceElement(LFRicCollection):
'''
Holds all information on the properties of the Reference Element
Expand Down Expand Up @@ -2156,23 +2159,29 @@ def colourmap_init(self):
# Array holding the last cell of a given colour.
if (Config.get().distributed_memory and

Check warning on line 2160 in src/psyclone/dynamo0p3.py

View check run for this annotation

Codecov / codecov/patch

src/psyclone/dynamo0p3.py#L2160

Added line #L2160 was not covered by tests
not call.all_updates_are_writes):
# This will require a loop into the halo and so the array is
# 2D (indexed by colour *and* halo depth).
# This will require a loop into the halo and so the array
# is 2D (indexed by colour *and* halo depth).
base_name = "last_halo_tile_per_colour_" + carg_name
last_tile = self._schedule.symbol_table.find_or_create_array(
base_name, 2, ScalarType.Intrinsic.INTEGER, tag=base_name)
base_name = "last_halo_cell_per_colour_and_tile_" + carg_name
last_cell_tile = self._schedule.symbol_table.find_or_create_array(
base_name, 3, ScalarType.Intrinsic.INTEGER, tag=base_name)
last_tile = sym_tab.find_or_create_array(

Check warning on line 2165 in src/psyclone/dynamo0p3.py

View check run for this annotation

Codecov / codecov/patch

src/psyclone/dynamo0p3.py#L2164-L2165

Added lines #L2164 - L2165 were not covered by tests
base_name, 2, ScalarType.Intrinsic.INTEGER,
tag=base_name)
base_name = ("last_halo_cell_per_colour_and_tile_" +

Check warning on line 2168 in src/psyclone/dynamo0p3.py

View check run for this annotation

Codecov / codecov/patch

src/psyclone/dynamo0p3.py#L2168

Added line #L2168 was not covered by tests
carg_name)
last_cell_tile = sym_tab.find_or_create_array(

Check warning on line 2170 in src/psyclone/dynamo0p3.py

View check run for this annotation

Codecov / codecov/patch

src/psyclone/dynamo0p3.py#L2170

Added line #L2170 was not covered by tests
base_name, 3, ScalarType.Intrinsic.INTEGER,
tag=base_name)
else:
# Array holding the last edge cell of a given colour. Just 1D
# as indexed by colour only.
# Array holding the last edge cell of a given colour. Just
# 1D as indexed by colour only.
base_name = "last_edge_tile_all_colours_" + carg_name
last_tile = self._schedule.symbol_table.find_or_create_array(
base_name, 1, ScalarType.Intrinsic.INTEGER, tag=base_name)
base_name = "last_edge_cell_all_colours_and_tiles_" + carg_name
last_cell_tile = self._schedule.symbol_table.find_or_create_array(
base_name, 2, ScalarType.Intrinsic.INTEGER, tag=base_name)
last_tile = sym_tab.find_or_create_array(

Check warning on line 2177 in src/psyclone/dynamo0p3.py

View check run for this annotation

Codecov / codecov/patch

src/psyclone/dynamo0p3.py#L2176-L2177

Added lines #L2176 - L2177 were not covered by tests
base_name, 1, ScalarType.Intrinsic.INTEGER,
tag=base_name)
base_name = ("last_edge_cell_all_colours_and_tiles_"

Check warning on line 2180 in src/psyclone/dynamo0p3.py

View check run for this annotation

Codecov / codecov/patch

src/psyclone/dynamo0p3.py#L2180

Added line #L2180 was not covered by tests
+ carg_name)
last_cell_tile = sym_tab.find_or_create_array(

Check warning on line 2182 in src/psyclone/dynamo0p3.py

View check run for this annotation

Codecov / codecov/patch

src/psyclone/dynamo0p3.py#L2182

Added line #L2182 was not covered by tests
base_name, 2, ScalarType.Intrinsic.INTEGER,
tag=base_name)
# Add these symbols into the dictionary entry for this
# inter-grid kernel
call._intergrid_ref.set_tilecolour_info(

Check warning on line 2187 in src/psyclone/dynamo0p3.py

View check run for this annotation

Codecov / codecov/patch

src/psyclone/dynamo0p3.py#L2187

Added line #L2187 was not covered by tests
Expand All @@ -2188,17 +2197,19 @@ def colourmap_init(self):
# Array holding the last cell of a given colour.
if (Config.get().distributed_memory and
not call.all_updates_are_writes):
# This will require a loop into the halo and so the array is
# 2D (indexed by colour *and* halo depth).
# This will require a loop into the halo and so the array
# is 2D (indexed by colour *and* halo depth).
base_name = "last_halo_cell_all_colours_" + carg_name
last_cell = self._schedule.symbol_table.find_or_create_array(
base_name, 2, ScalarType.Intrinsic.INTEGER, tag=base_name)
last_cell = sym_tab.find_or_create_array(
base_name, 2, ScalarType.Intrinsic.INTEGER,
tag=base_name)
else:
# Array holding the last edge cell of a given colour. Just 1D
# as indexed by colour only.
# Array holding the last edge cell of a given colour. Just
# 1D as indexed by colour only.
base_name = "last_edge_cell_all_colours_" + carg_name
last_cell = self._schedule.symbol_table.find_or_create_array(
base_name, 1, ScalarType.Intrinsic.INTEGER, tag=base_name)
last_cell = sym_tab.find_or_create_array(
base_name, 1, ScalarType.Intrinsic.INTEGER,
tag=base_name)
# Add these symbols into the dictionary entry for this
# inter-grid kernel
call._intergrid_ref.set_colour_info(colour_map, ncolours,
Expand Down Expand Up @@ -2458,8 +2469,14 @@ def initialise(self, parent):
parent.add(CommentGen(parent, " Get the tilecolourmap"))
parent.add(CommentGen(parent, ""))
# Look-up variable names for colourmap and number of colours
ntilecolours = self._schedule.symbol_table.find_or_create_tag(
"ntilecolour").name
colour_map = self._schedule.symbol_table.find_or_create_tag(
"tmap").name
# Get the number of colourtiles
parent.add(AssignGen(
parent, lhs=ntilecolours,
rhs=f"{mesh_name}%get_ntilecolours()"))
# Get the colour map
parent.add(AssignGen(
parent, pointer=True, lhs=colour_map,
Expand Down Expand Up @@ -2585,12 +2602,14 @@ def initialise(self, parent):
# Colour map for the coarse mesh (if required)
if dig.tilecolourmap_symbol:
# Number of colours
parent.add(AssignGen(parent, lhs=dig.ntilecolours_var_symbol.name,
parent.add(AssignGen(parent,

Check warning on line 2605 in src/psyclone/dynamo0p3.py

View check run for this annotation

Codecov / codecov/patch

src/psyclone/dynamo0p3.py#L2605

Added line #L2605 was not covered by tests
lhs=dig.ntilecolours_var_symbol.name,
rhs=coarse_mesh + "%get_ntilecolours()"))
# Colour map itself
parent.add(AssignGen(parent, lhs=dig.tilecolourmap_symbol.name,

Check warning on line 2609 in src/psyclone/dynamo0p3.py

View check run for this annotation

Codecov / codecov/patch

src/psyclone/dynamo0p3.py#L2609

Added line #L2609 was not covered by tests
pointer=True,
rhs=coarse_mesh + "%get_coloured_tiling_map()"))
rhs=(coarse_mesh +
"%get_coloured_tiling_map()")))
# Last halo/edge cell per colour.
# sym = dig.last_cell_tile_var_symbol
# if len(sym.datatype.shape) == 2:
Expand Down Expand Up @@ -2725,13 +2744,6 @@ def ncolours_var_symbol(self):
'''
return self._ncolours_var_symbol

@property
def ntilecolours_var_symbol(self):
''':returns: the symbol for storing the number of colours.
:rtype: :py:class:`psyclone.psyir.symbols.Symbol`
'''
return self._ntilecolours_var_symbol

@property
def last_cell_var_symbol(self):
''':returns: the last halo/edge cell variable.
Expand Down Expand Up @@ -4961,7 +4973,9 @@ def _compute_from_field(self, field):
else:
# loop redundant computation is to the maximum depth
self._max_depth = True
elif loop.upper_bound_name == "ncolour":
elif loop.upper_bound_name in ("ncolour",
"last_edge_tile_per_colour",
"last_edge_cell_per_coloured_tile"):
# Loop is coloured but does not access the halo.
pass
elif loop.upper_bound_name in ["ncells", "nannexed"]:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,10 +278,10 @@ def test_colour_trans_tiled(tmpdir, dist_mem):
else:
assert """
do colour = loop0_start, loop0_stop, 1
do tile = loop1_start, mesh%get_last_halo_tile_per_colour4\
(colour,max_halo_depth_mesh), 1
do tile = loop1_start, mesh%get_last_halo_tile_per_colour\
(colour,1), 1
do cell = loop2_start, mesh%get_last_halo_cell_per_colour_and_tile\
(colour,tile,max_halo_depth_mesh), 1
(colour,tile,1), 1
""" in gen


Expand Down
9 changes: 4 additions & 5 deletions src/psyclone/transformations.py
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,8 @@ def validate(self, node, options=None):
# colouring.
const = LFRicConstants()
if node.field_space.orig_name not in const.VALID_DISCONTINUOUS_NAMES:
if node.loop_type not in ('colour', 'colourtiles') and node.has_inc_arg():
if (node.loop_type not in ('colour', 'colourtiles')
and node.has_inc_arg()):
raise TransformationError(
f"Error in {self.name} transformation. The kernel has an "
f"argument with INC access but the loop is of type "
Expand Down Expand Up @@ -1242,7 +1243,6 @@ def _create_colours_loop(self, node):

return colours_loop


def _create_tiled_colours_loops(self, node):
'''
Creates a nested loop (colours, and cells of a given colour) which
Expand Down Expand Up @@ -1279,8 +1279,7 @@ def _create_tiled_colours_loops(self, node):
colour_loop.set_upper_bound("last_halo_tile_per_colour", index)
else:
# No halo access.
colour_loop.set_upper_bound("ncolour")
# colour_loop.set_upper_bound("error")
colour_loop.set_upper_bound("last_edge_tile_per_colour")

# Add this loop as a child of our loop over colours
colours_loop.loop_body.addchild(colour_loop)
Expand All @@ -1302,7 +1301,7 @@ def _create_tiled_colours_loops(self, node):
index)
else:
# No halo access.
tile_loop.set_upper_bound("ncells")
tile_loop.set_upper_bound("last_edge_cell_per_coloured_tile")

# Add this loop as a child of our loop over colours
colour_loop.loop_body.addchild(tile_loop)
Expand Down

0 comments on commit 1752893

Please sign in to comment.