Merge pull request #3258 from stfc/3250_complete_coverage

(closes #279) Remove Arguments.dofs and generally improve code coverage

Author: arporter

changelog

@@ -1,3 +1,6 @@
+   16) PR #3258 for #279. Removes Arguments.dofs and generally improves
+   testing coverage.
+
    15) PR #3260 for #2884. Allow WHERE with imported symbols by changing
    Ref2ArrayRangeTrans behaviour to do the validations.
 

codecov.yml

@@ -0,0 +1,6 @@
+coverage:
+  status:
+    project:
+      default:
+        removed_code_behavior: fully_covered_patch
+      

src/psyclone/core/access_sequence.py

@@ -47,7 +47,7 @@
 from psyclone.core.signature import Signature
 from psyclone.errors import InternalError
 
-if TYPE_CHECKING:  # pragma: no cover
+if TYPE_CHECKING:
     from psyclone.psyir.nodes import Node
     from psyclone.psyir.symbols import Symbol
 

src/psyclone/gocean1p0.py

@@ -1171,7 +1171,6 @@ def __init__(self, call, parent_call, check=True):
                 raise ParseError(f"Invalid kernel argument type. Found "
                                  f"'{arg.argument_type}' but must be one of "
                                  f"['grid_property', 'scalar', 'field'].")
-        self._dofs = []
 
     def psyir_expressions(self):
         '''
@@ -1207,13 +1206,6 @@ def find_grid_access(self):
         # if the kernel requires a grid-property argument.
         return None
 
-    @property
-    def dofs(self):
-        ''' Currently required for invoke base class although this makes no
-            sense for GOcean. Need to refactor the Invoke base class and
-            remove the need for this property (#279). '''
-        return self._dofs
-
     @property
     def acc_args(self):
         '''

src/psyclone/lfric.py

@@ -5416,8 +5416,6 @@ def __init__(self, call, parent_call, check=True):
                     )
                     arg.stencil.direction_arg.varname = symbol.name
 
-        self._dofs = []
-
         # Generate a static list of unique function-space names used
         # by the set of arguments: store the mangled names as these
         # are what we use at the level of an Invoke
@@ -5605,13 +5603,6 @@ def iteration_space_arg(self):
             "field, a modified operator, or an unmodified field (in the case "
             "of a modified scalar). None of these were found.")
 
-    @property
-    def dofs(self):
-        ''' Currently required for Invoke base class although this
-        makes no sense for LFRic. Need to refactor the Invoke base class
-        and remove the need for this property (#279). '''
-        return self._dofs
-
     def psyir_expressions(self):
         '''
         :returns: the PSyIR expressions representing this Argument list.

src/psyclone/psyGen.py

@@ -232,9 +232,6 @@ def container(self):
         '''
         return self._container
 
-    def __str__(self):
-        return "PSy"
-
     @property
     def invokes(self):
         ''':returns: the list of invokes.
@@ -310,9 +307,6 @@ def __init__(self, alg_calls, invoke_cls, psy):
             self.invoke_map[my_invoke.name] = my_invoke
             self.invoke_list.append(my_invoke)
 
-    def __str__(self):
-        return "Invokes object containing "+str(self.names)
-
     @property
     def psy(self):
         '''
@@ -431,17 +425,6 @@ def __init__(self, alg_invocation, idx, schedule_class, invokes):
                     # literals have no name
                     pass
 
-        # work out the unique dofs required in this subroutine
-        self._dofs = {}
-        for kern_call in self._schedule.coded_kernels():
-            dofs = kern_call.arguments.dofs
-            for dof in dofs:
-                if dof not in self._dofs:
-                    # Only keep the first occurrence for the moment. We will
-                    # need to change this logic at some point as we need to
-                    # cope with writes determining the dofs that are used.
-                    self._dofs[dof] = [kern_call, dofs[dof][0]]
-
     def __str__(self):
         return self._name+"("+", ".join([str(arg) for arg in
                                          self._alg_unique_args])+")"
@@ -850,65 +833,6 @@ def args(self):
         base method and simply return our argument. '''
         return [self._field]
 
-    def check_vector_halos_differ(self, node):
-        '''Helper method which checks that two halo exchange nodes (one being
-        self and the other being passed by argument) operating on the
-        same field, both have vector fields of the same size and use
-        different vector indices. If this is the case then the halo
-        exchange nodes do not depend on each other. If this is not the
-        case then an internal error will have occured and we raise an
-        appropriate exception.
-
-        :param node: a halo exchange which should exchange the same field as \
-                     self.
-        :type node: :py:class:`psyclone.psyGen.HaloExchange`
-        :raises GenerationError: if the argument passed is not a halo exchange.
-        :raises GenerationError: if the field name in the halo exchange \
-                                 passed in has a different name to the field \
-                                 in this halo exchange.
-        :raises GenerationError: if the field in this halo exchange is not a \
-                                 vector field
-        :raises GenerationError: if the vector size of the field in this halo \
-                                 exchange is different to vector size of the \
-                                 field in the halo exchange passed by argument.
-        :raises GenerationError: if the vector index of the field in this \
-                                 halo exchange is the same as the vector \
-                                 index of the field in the halo exchange \
-                                 passed by argument.
-
-        '''
-
-        if not isinstance(node, HaloExchange):
-            raise GenerationError(
-                "Internal error, the argument passed to "
-                "HaloExchange.check_vector_halos_differ() is not "
-                "a halo exchange object")
-
-        if self.field.name != node.field.name:
-            raise GenerationError(
-                f"Internal error, the halo exchange object passed to "
-                f"HaloExchange.check_vector_halos_differ() has a different "
-                f"field name '{node.field.name}' to self '{self.field.name}'")
-
-        if self.field.vector_size <= 1:
-            raise GenerationError(
-                "Internal error, HaloExchange.check_vector_halos_differ() "
-                "a halo exchange depends on another halo exchange but the "
-                f"vector size of field '{self.field.name}' is 1")
-
-        if self.field.vector_size != node.field.vector_size:
-            raise GenerationError(
-                f"Internal error, HaloExchange.check_vector_halos_differ() "
-                f"a halo exchange depends on another halo exchange but the "
-                f"vector sizes for field '{self.field.name}' differ")
-
-        if self.vector_index == node.vector_index:
-            raise GenerationError(
-                f"Internal error, HaloExchange.check_vector_halos_differ() "
-                f"a halo exchange depends on another halo exchange but both "
-                f"vector id's ('{self.vector_index}') of field "
-                f"'{self.field.name}' are the same")
-
     def node_str(self, colour=True):
         '''
         Returns the name of this node with (optional) control codes

src/psyclone/psyir/frontend/fparser2.py

@@ -5727,9 +5727,6 @@ def _program_handler(self, node, parent):
         # one) so this can't be provided as the name of the
         # FileContainer.
         file_container = FileContainer("None", parent=parent)
-        if len(node.children) == 1 and node.children[0] is None:
-            # We have an empty file
-            return file_container
         self.process_nodes(file_container, node.children)
         return file_container
 

src/psyclone/psyir/nodes/codeblock.py

@@ -213,10 +213,7 @@ def get_symbol_names(self) -> List[str]:
         # symbol table.
         for node in walk(parse_tree, Fortran2003.Directive):
             string_rep = node.tostr()
-            # Directives start with a $
-            if string_rep.lstrip()[0:2] != "!$":
-                continue
-            string_rep = string_rep[2:]
+            string_rep = string_rep[string_rep.index("$"):]
             pattern = pattern_tools.name.get_compiled()
             matches = re.findall(pattern, string_rep)
             scope = self.scope

src/psyclone/tests/conftest.py

@@ -81,7 +81,7 @@ def pytest_addoption(parser):
 
 
 @pytest.fixture
-def have_graphviz():
+def have_graphviz():  # pragma: no-cover
     ''' Whether or not the system has graphviz installed. This refers to
     the underlying system library, not the python bindings that are provided
     by 'import graphviz'. '''

src/psyclone/tests/domain/gocean/transformations/globalstoargs_test.py

@@ -38,10 +38,11 @@
 
 import os
 import pytest
+from psyclone.gocean1p0 import GOKern
 from psyclone.parse.algorithm import parse
 from psyclone.psyGen import PSyFactory, InvokeSchedule
 from psyclone.psyir.symbols import (DataSymbol, REAL_TYPE, INTEGER_TYPE,
-                                    CHARACTER_TYPE, Symbol)
+                                    CHARACTER_TYPE, Symbol, SymbolError)
 from psyclone.tests.utilities import get_invoke, make_external_module
 from psyclone.transformations import (KernelImportsToArguments,
                                       TransformationError)
@@ -87,6 +88,30 @@ def test_kernelimportsstoargumentstrans_no_outer_module_import():
             "module scope." in str(err.value))
 
 
+def test_kernelimportsstoargumentstrans_unsuccessful_get_callees(monkeypatch):
+    ''' Check that the validation produces the correct error when a
+    get_callees method is unsuccessful.
+    '''
+    trans = KernelImportsToArguments()
+    path = os.path.join(BASEPATH, "gocean1p0")
+    _, invoke_info = parse(os.path.join(path,
+                                        "single_invoke_kern_with_global.f90"),
+                           api=API)
+    psy = PSyFactory(API).create(invoke_info)
+    invoke = psy.invokes.invoke_list[0]
+    kernel = invoke.schedule.coded_kernels()[0]
+
+    # Monkeypatch get_callees to always produce an error.
+    def raise_error(_):
+        raise SymbolError("some error")
+
+    monkeypatch.setattr(GOKern, "get_callees", raise_error)
+    with pytest.raises(TransformationError) as err:
+        trans.validate(kernel)
+    assert ("Kernel 'kernel_with_global_code' contains undeclared symbol:"
+            in str(err.value))
+
+
 def test_kernelimportstoargumentstrans_no_wildcard_import():
     ''' Check that the transformation rejects kernels with wildcard
     imports. '''