From 97a3e861c732438497e991947e3c9bd68be55fdc Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Wed, 7 May 2025 10:59:21 +1000 Subject: [PATCH 1/9] #2923 Support binary operations in SymPy. --- src/psyclone/psyir/backend/sympy_writer.py | 25 +++++++++++++ .../tests/core/symbolic_maths_test.py | 37 +++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/src/psyclone/psyir/backend/sympy_writer.py b/src/psyclone/psyir/backend/sympy_writer.py index 5b261638d8..623bd8a05c 100644 --- a/src/psyclone/psyir/backend/sympy_writer.py +++ b/src/psyclone/psyir/backend/sympy_writer.py @@ -697,6 +697,31 @@ def reference_node(self, node): return (f"{name}{self.array_parenthesis[0]}" f"{','.join(result)}{self.array_parenthesis[1]}") + # ------------------------------------------------------------------------ + def binaryoperation_node(self, node: BinaryOperation) -> str: + '''This function converts logical binary operations into + SymPy format. Non-logical binary operations have the same + representation otherwise, so it calls the base class. + + :param node: a Reference PSyIR BinaryOperation. + + ''' + + for psy_op, sympy_op in [(BinaryOperation.Operator.AND, + "{lhs} & {rhs}"), + (BinaryOperation.Operator.OR, + "{lhs} | {rhs}"), + (BinaryOperation.Operator.EQV, + "Equivalent({lhs}, {rhs})"), + (BinaryOperation.Operator.NEQV, + "~Equivalent({lhs}, {rhs})")]: + if node.operator == psy_op: + lhs = self._visit(node.children[0]) + rhs = self._visit(node.children[1]) + return sympy_op.format(rhs=rhs, lhs=lhs) + + return super().binaryoperation_node(node) + # ------------------------------------------------------------------------ def gen_indices(self, indices, var_name=None): '''Given a list of PSyIR nodes representing the dimensions of an diff --git a/src/psyclone/tests/core/symbolic_maths_test.py b/src/psyclone/tests/core/symbolic_maths_test.py index 00d96a3c3d..b6f96dd7fd 100644 --- a/src/psyclone/tests/core/symbolic_maths_test.py +++ b/src/psyclone/tests/core/symbolic_maths_test.py @@ -512,3 +512,40 @@ def test_symbolic_maths_array_and_array_index(fortran_reader): assert sym_maths.equal(psyir.children[0][1].rhs, psyir.children[0][1].rhs) + + +@pytest.mark.parametrize("expressions", [(".false. .and. .false.", "False"), + (".false. .and. .true.", "False"), + (".true. .and. .false.", "False"), + (".true. .and. .true.", "True"), + (".false. .or. .false.", "False"), + (".false. .or. .true.", "True"), + (".true. .or. .false.", "True"), + (".true. .or. .true.", "True"), + (".false. .eqv. .false.", "True"), + (".false. .eqv. .true.", "False"), + (".true. .eqv. .false.", "False"), + (".true. .eqv. .true.", "True"), + (".false. .neqv. .false.", "False"), + (".false. .neqv. .true.", "True"), + (".true. .neqv. .false.", "True"), + (".true. .neqv. .true.", "False"), + ]) +def test_sym_writer_boolean_expr(fortran_reader, expressions): + '''Test that booleans are written in the way that SymPy accepts. + ''' + # A dummy program to easily create the PSyIR for the + # expressions we need. We just take the RHS of the assignments + source = f'''program test_prog + logical :: bool_expr + bool_expr = {expressions[0]} + bool_expr = {expressions[1]} + end program test_prog ''' + + psyir = fortran_reader.psyir_from_source(source) + lit0 = psyir.children[0].children[0].rhs + lit1 = psyir.children[0].children[1].rhs + sympy_writer = SymPyWriter() + + sympy_expr = sympy_writer(lit0) + assert sympy_expr == sympy_writer(lit1) From 65221a9b47aa3671675adf8c00759842058e738e Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Wed, 7 May 2025 11:23:52 +1000 Subject: [PATCH 2/9] #2923 Added type information. --- src/psyclone/psyir/backend/sympy_writer.py | 116 +++++++++++---------- 1 file changed, 59 insertions(+), 57 deletions(-) diff --git a/src/psyclone/psyir/backend/sympy_writer.py b/src/psyclone/psyir/backend/sympy_writer.py index 623bd8a05c..c88ff5ffe6 100644 --- a/src/psyclone/psyir/backend/sympy_writer.py +++ b/src/psyclone/psyir/backend/sympy_writer.py @@ -39,15 +39,19 @@ ''' import keyword +from typing import Dict, List, Optional, Set, Union import sympy from sympy.parsing.sympy_parser import parse_expr from psyclone.psyir.backend.fortran import FortranWriter from psyclone.psyir.backend.visitor import VisitorError +from psyclone.core import Signature from psyclone.psyir.frontend.sympy_reader import SymPyReader from psyclone.psyir.nodes import ( - DataNode, Range, Reference, IntrinsicCall, Call) + ArrayOfStructuresReference, ArrayReference, BinaryOperation, Call, + DataNode, IntrinsicCall, Literal, Node, + Range, Reference, StructureReference) from psyclone.psyir.symbols import (ArrayType, ScalarType, SymbolTable) @@ -96,7 +100,7 @@ class SymPyWriter(FortranWriter): # A list of all reserved Python keywords (Fortran variables that are the # same as a reserved name must be renamed, otherwise parsing will fail). # This class attribute will get initialised in __init__: - _RESERVED_NAMES = set() + _RESERVED_NAMES: Set[str] = set() def __init__(self): super().__init__() @@ -197,7 +201,11 @@ def __getitem__(self, _): "never be called.") # ------------------------------------------------------------------------- - def _create_sympy_array_function(self, name, sig=None, num_dims=None): + def _create_sympy_array_function( + self, + name: str, + sig: Optional[Signature] = None, + num_dims: Optional[List[int]] = None) -> sympy.Function: '''Creates a Function class with the given name to be used for SymPy parsing. This Function overwrites the conversion to string, and will replace the triplicated array indices back to the normal Fortran @@ -206,7 +214,7 @@ def _create_sympy_array_function(self, name, sig=None, num_dims=None): to the object, so that the SymPyReader can recreate the proper access to a user-defined type. - :param str name: name of the function class to create. + :param name: name of the function class to create. :param sig: the signature of the variable, which is required to convert user defined types back properly. Only defined for user-defined types. @@ -246,8 +254,10 @@ def _create_sympy_array_function(self, name, sig=None, num_dims=None): return new_func # ------------------------------------------------------------------------- - def _create_type_map(self, list_of_expressions, identical_variables=None, - all_variables_positive=None): + def _create_type_map(self, + list_of_expressions: List[Node], + identical_variables: Optional[Dict[str, str]] = None, + all_variables_positive: Optional[bool] = None): '''This function creates a dictionary mapping each Reference in any of the expressions to either a SymPy Function (if the reference is an array reference) or a Symbol (if the reference is not an @@ -282,11 +292,9 @@ def _create_type_map(self, list_of_expressions, identical_variables=None, :param list_of_expressions: the list of expressions from which all references are taken and added to a symbol table to avoid renaming any symbols (so that only member names will be renamed). - :type list_of_expressions: List[:py:class:`psyclone.psyir.nodes.Node`] :param identical_variables: which variable names are known to represent identical quantities. - :type identical_variables: Optional[dict[str, str]] - :param Optional[bool] all_variables_positive: whether or not (the + :param all_variables_positive: whether or not (the default) to assume that all variables are positive definite quantities. @@ -363,25 +371,24 @@ def _create_type_map(self, list_of_expressions, identical_variables=None, # ------------------------------------------------------------------------- @property - def lower_bound(self): + def lower_bound(self) -> str: ''':returns: the name to be used for an unspecified lower bound. - :rtype: str ''' return self._lower_bound # ------------------------------------------------------------------------- @property - def upper_bound(self): + def upper_bound(self) -> str: ''':returns: the name to be used for an unspecified upper bound. - :rtype: str ''' return self._upper_bound # ------------------------------------------------------------------------- @property - def type_map(self): + def type_map(self) -> Dict[str, Union[sympy.core.symbol.Symbol, + sympy.core.function.Function]]: ''':returns: the mapping of names to SymPy symbols or functions. :rtype: Dict[str, Union[:py:class:`sympy.core.symbol.Symbol`, :py:class:`sympy.core.function.Function`]] @@ -390,8 +397,12 @@ def type_map(self): return self._sympy_type_map # ------------------------------------------------------------------------- - def _to_str(self, list_of_expressions, identical_variables=None, - all_variables_positive=False): + def _to_str( + self, + list_of_expressions: Union[Node, List[Node]], + identical_variables: Optional[Dict[str, str]] = None, + all_variables_positive: Optional[bool] = False) -> Union[str, + List[str]]: '''Converts PSyIR expressions to strings. It will replace Fortran- specific expressions with code that can be parsed by SymPy. The argument can either be a single element (in which case a single string @@ -403,24 +414,23 @@ def _to_str(self, list_of_expressions, identical_variables=None, :param identical_variables: which variable names are known to be identical - :type identical_variables: Optional[dict[str, str]] - :param list_of_expressions: the list of expressions which are to be converted into SymPy-parsable strings. - :type list_of_expressions: Union[:py:class:`psyclone.psyir.nodes.Node`, - List[:py:class:`psyclone.psyir.nodes.Node`]] - :param Optional[bool] all_variables_positive: whether or not (the + :param all_variables_positive: whether or not (the default) to assume that all variables are positive definite quantities. :returns: the converted strings(s). - :rtype: Union[str, List[str]] ''' is_list = isinstance(list_of_expressions, (tuple, list)) if not is_list: + # Make mypy happy: + assert isinstance(list_of_expressions, Node) list_of_expressions = [list_of_expressions] + # Make mypy happy: + assert isinstance(list_of_expressions, List) # Create the type map in `self._sympy_type_map`, which is required # when converting these strings to SymPy expressions self._create_type_map(list_of_expressions, @@ -438,8 +448,13 @@ def _to_str(self, list_of_expressions, identical_variables=None, return expression_str_list # ------------------------------------------------------------------------- - def __call__(self, list_of_expressions, identical_variables=None, - all_variables_positive=False): + def __call__( + self, + list_of_expressions: Union[Node, List[Node]], + identical_variables: Optional[Dict[str, str]] = None, + all_variables_positive: Optional[bool] = False) \ + -> Union[sympy.core.basic.Basic, + List[sympy.core.basic.Basic]]: ''' This function takes a list of PSyIR expressions, and converts them all into Sympy expressions using the SymPy parser. @@ -454,11 +469,8 @@ def __call__(self, list_of_expressions, identical_variables=None, :param list_of_expressions: the list of expressions which are to be converted into SymPy-parsable strings. - :type list_of_expressions: list of - :py:class:`psyclone.psyir.nodes.Node` :param identical_variables: which variable names are known to be identical - :type identical_variables: Optional[dict[str, str]] :param Optional[bool] all_variables_positive: whether or not (the default) to assume that all variables are positive definite quantities. @@ -466,8 +478,6 @@ def __call__(self, list_of_expressions, identical_variables=None, :returns: a 2-tuple consisting of the the converted PSyIR expressions, followed by a dictionary mapping the symbol names to SymPy Symbols. - :rtype: Union[:py:class:`sympy.core.basic.Basic`, - List[:py:class:`sympy.core.basic.Basic`]] :raises VisitorError: if an invalid SymPy expression is found. :raises TypeError: if the identical_variables parameter is not @@ -486,7 +496,9 @@ def __call__(self, list_of_expressions, identical_variables=None, is_list = isinstance(list_of_expressions, (tuple, list)) if not is_list: + assert isinstance(list_of_expressions, Node) list_of_expressions = [list_of_expressions] + expression_str_list = self._to_str( list_of_expressions, identical_variables=identical_variables, all_variables_positive=all_variables_positive) @@ -506,39 +518,39 @@ def __call__(self, list_of_expressions, identical_variables=None, return result[0] # ------------------------------------------------------------------------- - def arrayreference_node(self, node): + def arrayreference_node(self, node: ArrayReference) -> str: '''The implementation of the method handling a ArrayOfStructureReference is generic enough to also handle non-structure arrays. So just use it. :param node: a ArrayReference PSyIR node. - :type node: :py:class:`psyclone.psyir.nodes.ArrayReference` :returns: the code as string. - :rtype: str ''' return self.arrayofstructuresreference_node(node) # ------------------------------------------------------------------------- - def structurereference_node(self, node): + def structurereference_node(self, node: StructureReference) -> str: '''The implementation of the method handling a ArrayOfStructureReference is generic enough to also handle non-arrays. So just use it. :param node: a StructureReference PSyIR node. - :type node: :py:class:`psyclone.psyir.nodes.StructureReference` :returns: the code as string. - :rtype: str ''' return self.arrayofstructuresreference_node(node) # ------------------------------------------------------------------------- - def arrayofstructuresreference_node(self, node): - '''This handles ArrayOfStructureReferences (and also simple - StructureReferences). An access like ``a(i)%b(j)`` is converted to + def arrayofstructuresreference_node( + self, + node: Union[ArrayOfStructuresReference, + ArrayReference, + StructureReference]) -> str: + '''This handles ArrayOfStructuresReference (and also simple + StructureReference). An access like ``a(i)%b(j)`` is converted to the string ``a_b(i,i,1,j,j,1)`` (also handling name clashes in case that the user code already contains a symbol ``a_b``). The SymPy function created for this new symbol will store the original signature @@ -548,10 +560,8 @@ def arrayofstructuresreference_node(self, node): representation :param node: a StructureReference PSyIR node. - :type node: :py:class:`psyclone.psyir.nodes.StructureReference` :returns: the code as string. - :rtype: str ''' sig, indices = node.get_signature_and_indices() @@ -591,7 +601,7 @@ def arrayofstructuresreference_node(self, node): return unique_name # ------------------------------------------------------------------------- - def literal_node(self, node): + def literal_node(self, node: Literal) -> str: '''This method is called when a Literal instance is found in the PSyIR tree. For SymPy we need to handle booleans (which are expected to be capitalised: True). Real values work by just ignoring any precision @@ -599,10 +609,8 @@ def literal_node(self, node): and will raise an exception. :param node: a Literal PSyIR node. - :type node: :py:class:`psyclone.psyir.nodes.Literal` :returns: the SymPy representation for the literal. - :rtype: str :raises TypeError: if a character constant is found, which is not supported with SymPy. @@ -621,17 +629,16 @@ def literal_node(self, node): # information can be ignored. return node.value - def intrinsiccall_node(self, node): + # ------------------------------------------------------------------------- + def intrinsiccall_node(self, node: IntrinsicCall) -> str: ''' This method is called when an IntrinsicCall instance is found in the PSyIR tree. The Sympy backend will use the exact sympy name for some math intrinsics (listed in _intrinsic_to_str) and will remove named arguments. :param node: an IntrinsicCall PSyIR node. - :type node: :py:class:`psyclone.psyir.nodes.IntrinsicCall` :returns: the SymPy representation for the Intrinsic. - :rtype: str ''' # Sympy does not support argument names, remove them for now @@ -660,7 +667,7 @@ def intrinsiccall_node(self, node): return super().call_node(node) # ------------------------------------------------------------------------- - def reference_node(self, node): + def reference_node(self, node: Reference) -> str: '''This method is called when a Reference instance is found in the PSyIR tree. It handles the case that this normal reference might be an array expression, which in the SymPy writer needs to have @@ -668,10 +675,8 @@ def reference_node(self, node): ``a`` to ``a(sympy_lower, sympy_upper, 1)``. :param node: a Reference PSyIR node. - :type node: :py:class:`psyclone.psyir.nodes.Reference` :returns: the text representation of this reference. - :rtype: str ''' # Support renaming a symbol (e.g. if it is a reserved Python name). @@ -706,7 +711,6 @@ def binaryoperation_node(self, node: BinaryOperation) -> str: :param node: a Reference PSyIR BinaryOperation. ''' - for psy_op, sympy_op in [(BinaryOperation.Operator.AND, "{lhs} & {rhs}"), (BinaryOperation.Operator.OR, @@ -723,7 +727,9 @@ def binaryoperation_node(self, node: BinaryOperation) -> str: return super().binaryoperation_node(node) # ------------------------------------------------------------------------ - def gen_indices(self, indices, var_name=None): + def gen_indices(self, + indices: List[Node], + var_name: Optional[str] = None): '''Given a list of PSyIR nodes representing the dimensions of an array, return a list of strings representing those array dimensions. This is used both for array references and array declarations. Note @@ -732,12 +738,10 @@ def gen_indices(self, indices, var_name=None): each array index into three parameters to support array expressions. :param indices: list of PSyIR nodes. - :type indices: List[:py:class:`psyclone.psyir.symbols.Node`] - :param str var_name: name of the variable for which the dimensions + :param var_name: name of the variable for which the dimensions are created. Not used in this implementation. :returns: the Fortran representation of the dimensions. - :rtype: List[str] :raises NotImplementedError: if the format of the dimension is not supported. @@ -770,16 +774,14 @@ def gen_indices(self, indices, var_name=None): return dims # ------------------------------------------------------------------------- - def range_node(self, node): + def range_node(self, node: Range) -> str: '''This method is called when a Range instance is found in the PSyIR tree. This implementation converts a range into three parameters for the corresponding SymPy function. :param node: a Range PSyIR node. - :type node: :py:class:`psyclone.psyir.nodes.Range` :returns: the Fortran code as a string. - :rtype: str ''' if node.parent and node.parent.is_lower_bound( From 600311a21be69804e511a2eebffebbe5df63cff3 Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Thu, 8 May 2025 23:19:29 +1000 Subject: [PATCH 3/9] #2923 Improved conversion by replacing loop with dictionary. --- src/psyclone/psyir/backend/sympy_writer.py | 35 +++++++++++----------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/psyclone/psyir/backend/sympy_writer.py b/src/psyclone/psyir/backend/sympy_writer.py index c88ff5ffe6..e986787c2e 100644 --- a/src/psyclone/psyir/backend/sympy_writer.py +++ b/src/psyclone/psyir/backend/sympy_writer.py @@ -39,7 +39,7 @@ ''' import keyword -from typing import Dict, List, Optional, Set, Union +from typing import Dict, Iterable, List, Optional, Set, Union import sympy from sympy.parsing.sympy_parser import parse_expr @@ -102,6 +102,14 @@ class SymPyWriter(FortranWriter): # This class attribute will get initialised in __init__: _RESERVED_NAMES: Set[str] = set() + # A mapping of PSyIR's logical binary operations to the required + # SymPy format: + _BINARY_OP_MAPPING: Dict[BinaryOperation.Operator, str] = \ + {BinaryOperation.Operator.AND: "And({lhs}, {rhs})", + BinaryOperation.Operator.OR: "Or({lhs}, {rhs})", + BinaryOperation.Operator.EQV: "Equivalent({lhs}, {rhs})", + BinaryOperation.Operator.NEQV: "Xor({lhs}, {rhs})"} + def __init__(self): super().__init__() @@ -255,7 +263,7 @@ def _create_sympy_array_function( # ------------------------------------------------------------------------- def _create_type_map(self, - list_of_expressions: List[Node], + list_of_expressions: Iterable[Node], identical_variables: Optional[Dict[str, str]] = None, all_variables_positive: Optional[bool] = None): '''This function creates a dictionary mapping each Reference in any @@ -399,7 +407,7 @@ def type_map(self) -> Dict[str, Union[sympy.core.symbol.Symbol, # ------------------------------------------------------------------------- def _to_str( self, - list_of_expressions: Union[Node, List[Node]], + list_of_expressions: Union[Node, Iterable[Node]], identical_variables: Optional[Dict[str, str]] = None, all_variables_positive: Optional[bool] = False) -> Union[str, List[str]]: @@ -423,14 +431,14 @@ def _to_str( :returns: the converted strings(s). ''' - is_list = isinstance(list_of_expressions, (tuple, list)) + is_list = isinstance(list_of_expressions, (Iterable)) if not is_list: # Make mypy happy: assert isinstance(list_of_expressions, Node) list_of_expressions = [list_of_expressions] # Make mypy happy: - assert isinstance(list_of_expressions, List) + assert isinstance(list_of_expressions, Iterable) # Create the type map in `self._sympy_type_map`, which is required # when converting these strings to SymPy expressions self._create_type_map(list_of_expressions, @@ -711,18 +719,11 @@ def binaryoperation_node(self, node: BinaryOperation) -> str: :param node: a Reference PSyIR BinaryOperation. ''' - for psy_op, sympy_op in [(BinaryOperation.Operator.AND, - "{lhs} & {rhs}"), - (BinaryOperation.Operator.OR, - "{lhs} | {rhs}"), - (BinaryOperation.Operator.EQV, - "Equivalent({lhs}, {rhs})"), - (BinaryOperation.Operator.NEQV, - "~Equivalent({lhs}, {rhs})")]: - if node.operator == psy_op: - lhs = self._visit(node.children[0]) - rhs = self._visit(node.children[1]) - return sympy_op.format(rhs=rhs, lhs=lhs) + if node.operator in self._BINARY_OP_MAPPING: + lhs = self._visit(node.children[0]) + rhs = self._visit(node.children[1]) + return self._BINARY_OP_MAPPING[node.operator].format(rhs=rhs, + lhs=lhs) return super().binaryoperation_node(node) From e5281220e2e0afb3fab53f53ca783e0f664dcd2f Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Thu, 8 May 2025 23:23:04 +1000 Subject: [PATCH 4/9] #2923 Convert logical sympy expressions back to PSyIR. --- src/psyclone/psyir/frontend/sympy_reader.py | 31 ++++++++++++++++++- .../tests/psyir/frontend/sympy_reader_test.py | 14 ++++++++- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/psyclone/psyir/frontend/sympy_reader.py b/src/psyclone/psyir/frontend/sympy_reader.py index 912636e715..fbb46dbc34 100644 --- a/src/psyclone/psyir/frontend/sympy_reader.py +++ b/src/psyclone/psyir/frontend/sympy_reader.py @@ -37,10 +37,37 @@ '''PSyIR frontend to convert a SymPy expression to PSyIR ''' +from sympy.printing.printer import Printer from psyclone.psyir.frontend.fortran import FortranReader +# pylint: disable=invalid-name +class FortranPrinter(Printer): + '''A helper class that converts Fortran logical operators back + to Fortran format (while SymPy has a Fortran printer (fcode) does this + as well, it does not handle e.g. Fortran Array expressions (a(2:5)), + so we need to use the not-Fortran-aware output to a normal string, + but handle logical operators separately. + ''' + def _print_And(self, expr): + '''Called when converting an AND expression.''' + return f"({'.AND.' .join(self._print(i) for i in expr.args)})" + + def _print_Or(self, expr): + '''Called when converting an OR expression.''' + return f"({'.OR.' .join(self._print(i) for i in expr.args)})" + + def _print_Equivalent(self, expr): + '''Called when converting an EQUIVALENT expression.''' + return f"({'.EQV.' .join(self._print(i) for i in expr.args)})" + + def _print_Xor(self, expr): + '''Called when converting an XOR expression, which in Fortran + is NEQV.''' + return f"({'.NEQV.' .join(self._print(i) for i in expr.args)})" + + class SymPyReader(): '''This class converts a SymPy expression, that was created by the SymPyWriter, back to PSyIR. It basically allows to use SymPy to modify @@ -121,7 +148,9 @@ def psyir_from_expression(self, sympy_expr, symbol_table): ''' # Convert the new sympy expression to PSyIR reader = FortranReader() - return reader.psyir_from_expression(str(sympy_expr), symbol_table) + fp = FortranPrinter() + return reader.psyir_from_expression(fp.doprint(sympy_expr), + symbol_table) # ------------------------------------------------------------------------- # pylint: disable=no-self-argument, too-many-branches diff --git a/src/psyclone/tests/psyir/frontend/sympy_reader_test.py b/src/psyclone/tests/psyir/frontend/sympy_reader_test.py index 539686b51c..1e6ae3178d 100644 --- a/src/psyclone/tests/psyir/frontend/sympy_reader_test.py +++ b/src/psyclone/tests/psyir/frontend/sympy_reader_test.py @@ -73,6 +73,18 @@ def test_sympy_reader_constructor(): ("b(:5:2)", "b(:5:2)"), ("b(2:5:1)", "b(2:5)"), ("b(2:5:2)", "b(2:5:2)"), + ("i .and. j", "i .AND. j"), + ("i .and. j .and. k", + "i .AND. j .AND. k"), + ("i .or. j", "i .OR. j"), + # Precedence requires the () + ("i .and. (i .or. j)", + "i .AND. (i .OR. j)"), + # Precedence rules discard the () + ("i .or. (i .and. j)", + "i .OR. i .AND. j"), + ("i .eqv. j", "i .EQV. j"), + ("i .neqv. j", "i .NEQV. j"), ]) def test_sympy_psyir_from_expression(fortran_reader, fortran_writer, expressions): @@ -87,7 +99,7 @@ def test_sympy_psyir_from_expression(fortran_reader, fortran_writer, ''' source = f'''program test_prog use my_mod - integer :: i, j + integer :: i, j, k integer :: a, b(10), c(10, 10) type(my_mod_type) :: d, e(10) x = {expressions[0]} From bc8bd1faab5c89855caa2fb38bf90139c08149fa Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Sat, 17 May 2025 01:02:16 +1000 Subject: [PATCH 5/9] #2923 Added Hugo's example. --- .../tests/core/symbolic_maths_test.py | 68 ++++++++++++++----- 1 file changed, 51 insertions(+), 17 deletions(-) diff --git a/src/psyclone/tests/core/symbolic_maths_test.py b/src/psyclone/tests/core/symbolic_maths_test.py index b6f96dd7fd..67a0fa3233 100644 --- a/src/psyclone/tests/core/symbolic_maths_test.py +++ b/src/psyclone/tests/core/symbolic_maths_test.py @@ -514,23 +514,26 @@ def test_symbolic_maths_array_and_array_index(fortran_reader): psyir.children[0][1].rhs) -@pytest.mark.parametrize("expressions", [(".false. .and. .false.", "False"), - (".false. .and. .true.", "False"), - (".true. .and. .false.", "False"), - (".true. .and. .true.", "True"), - (".false. .or. .false.", "False"), - (".false. .or. .true.", "True"), - (".true. .or. .false.", "True"), - (".true. .or. .true.", "True"), - (".false. .eqv. .false.", "True"), - (".false. .eqv. .true.", "False"), - (".true. .eqv. .false.", "False"), - (".true. .eqv. .true.", "True"), - (".false. .neqv. .false.", "False"), - (".false. .neqv. .true.", "True"), - (".true. .neqv. .false.", "True"), - (".true. .neqv. .true.", "False"), - ]) +@pytest.mark.parametrize( + "expressions", + [(".false. .and. .false.", "False"), + (".false. .and. .true.", "False"), + (".true. .and. .false.", "False"), + (".true. .and. .true.", "True"), + (".false. .or. .false.", "False"), + (".false. .or. .true.", "True"), + (".true. .or. .false.", "True"), + (".true. .or. .true.", "True"), + (".false. .eqv. .false.", "True"), + (".false. .eqv. .true.", "False"), + (".true. .eqv. .false.", "False"), + (".true. .eqv. .true.", "True"), + (".false. .neqv. .false.", "False"), + (".false. .neqv. .true.", "True"), + (".true. .neqv. .false.", "True"), + (".true. .neqv. .true.", "False"), + (" .false. .and. ((3 -2 + 4 - 5) .eq. 0 .and. .false.)", False), + ]) def test_sym_writer_boolean_expr(fortran_reader, expressions): '''Test that booleans are written in the way that SymPy accepts. ''' @@ -549,3 +552,34 @@ def test_sym_writer_boolean_expr(fortran_reader, expressions): sympy_expr = sympy_writer(lit0) assert sympy_expr == sympy_writer(lit1) + + +@pytest.mark.parametrize( + "expressions", + [(".true. .and. .false.", False), + (".true. .and. .true.", True), + (".false. .or. .true.", True), + ("3 .eq. 3", True), + (" ((3 -2 + 4 - 5) .eq. 0 .and. .false.) .or. .true.", True), + (" ((3 -2 + 4 - 5) .eq. 0 .and. .true.)", True), + (" (3 -2 + 4 - 5) .eq. 0 .and. .false. .and. .true.", False), + (" ((3 -2 + 4 - 5) .eq. 0 .and. .false.) .and. .true.", False), + (" .false. .and. ((3 -2 + 4 - 5) .eq. 0 .and. .false.)", False), + (" (((3 -2 + 4 - 5) .eq. 0) .and. .false.)", False), + ]) +def test_sym_writer_boolean_expr_add_test(fortran_reader, expressions): + '''Test that booleans are written in the way that SymPy accepts. + ''' + # A dummy program to easily create the PSyIR for the + # expressions we need. We just take the RHS of the assignments + source = f'''program test_prog + logical :: bool_expr + bool_expr = {expressions[0]} + end program test_prog ''' + + psyir = fortran_reader.psyir_from_source(source) + lit = psyir.children[0].children[0].rhs + sympy_writer = SymPyWriter() + sympy_expr = sympy_writer(lit) + print("XX", sympy_expr) + assert sympy_expr == expressions[1] From 73417dc502049e42a5f50f2457446c009b3da70e Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Sat, 17 May 2025 01:02:38 +1000 Subject: [PATCH 6/9] #2923 Fixed coverage. --- .../tests/psyir/backend/sympy_writer_test.py | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/psyclone/tests/psyir/backend/sympy_writer_test.py b/src/psyclone/tests/psyir/backend/sympy_writer_test.py index 99572651fc..afda518f43 100644 --- a/src/psyclone/tests/psyir/backend/sympy_writer_test.py +++ b/src/psyclone/tests/psyir/backend/sympy_writer_test.py @@ -514,6 +514,37 @@ def test_sympy_writer_user_types(fortran_reader, fortran_writer, assert fortran_writer(new_psyir) == fortran_expr +@pytest.mark.parametrize("fortran_expr,sympy_str", + [("a .and. b", "And(a, b)"), + ("a .or. b", "Or(a, b)"), + ("a .eqv. b", "Equivalent(a, b)"), + ("a .neqv. b", "Xor(a, b)"), + ]) +def test_sympy_writer_logicals(fortran_reader, fortran_writer, + fortran_expr, sympy_str): + '''Test handling of user-defined types, e.g. conversion of + ``a(i)%b(j)`` to ``a_b(i,i,1,j,j,1)``. Each Fortran expression + ``fortran_expr`` is first converted to a string ``sympy_str`` to be + parsed by SymPy. The sympy expression is then converted back to PSyIR. + This string must be the same as the original ``fortran_expr``. + + ''' + source = f'''program test_prog + logical :: a, b, x + x = {fortran_expr} + end program test_prog''' + + psyir = fortran_reader.psyir_from_source(source) + # Get the actual fortran expression requested: + psyir_expr = psyir.children[0].children[0].rhs + + # Convert the PSyIR to a SymPy string: + sympy_writer = SymPyWriter() + out = sympy_writer._to_str([psyir_expr]) + # Make sure we get the expected string as output: + assert out[0] == sympy_str + + @pytest.mark.parametrize("expression", ["def", "if", "raise", "del", "import", "return", "elif", "in", "try", "and", "else", "is", "while", From e9109e07bdb77f8b295e0cd75063fd958ea0488c Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Sat, 5 Jul 2025 01:34:19 +1000 Subject: [PATCH 7/9] #2923 Addressed comments from reviewer. --- src/psyclone/psyir/backend/sympy_writer.py | 59 ++++++++++----------- src/psyclone/psyir/frontend/sympy_reader.py | 12 ++--- 2 files changed, 34 insertions(+), 37 deletions(-) diff --git a/src/psyclone/psyir/backend/sympy_writer.py b/src/psyclone/psyir/backend/sympy_writer.py index de58f26249..a53b602c78 100644 --- a/src/psyclone/psyir/backend/sympy_writer.py +++ b/src/psyclone/psyir/backend/sympy_writer.py @@ -39,7 +39,7 @@ ''' import keyword -from typing import Dict, Iterable, List, Optional, Set, Union +from typing import Iterable, Optional, Union import sympy from sympy.parsing.sympy_parser import parse_expr @@ -103,11 +103,11 @@ class SymPyWriter(FortranWriter): # A list of all reserved Python keywords (Fortran variables that are the # same as a reserved name must be renamed, otherwise parsing will fail). # This class attribute will get initialised in __init__: - _RESERVED_NAMES: Set[str] = set() + _RESERVED_NAMES: set[str] = set() # A mapping of PSyIR's logical binary operations to the required # SymPy format: - _BINARY_OP_MAPPING: Dict[BinaryOperation.Operator, str] = \ + _BINARY_OP_MAPPING: dict[BinaryOperation.Operator, str] = \ {BinaryOperation.Operator.AND: "And({lhs}, {rhs})", BinaryOperation.Operator.OR: "Or({lhs}, {rhs})", BinaryOperation.Operator.EQV: "Equivalent({lhs}, {rhs})", @@ -190,7 +190,7 @@ def __new__(cls, *expressions): :returns: either an instance of SymPyWriter, if no parameter is specified, or a list of SymPy expressions. :rtype: Union[:py:class:`psyclone.psyir.backend.SymPyWriter`, - List[:py:class:`sympy.core.basic.Basic`]] + list[:py:class:`sympy.core.basic.Basic`]] ''' if expressions: @@ -214,11 +214,12 @@ def __getitem__(self, _): "never be called.") # ------------------------------------------------------------------------- - def _create_sympy_array_function(self, - name: str, - sig: Optional[Signature] = None, - num_dims: Optional[List[int]] = None, - is_call: Optional[bool] = False): + def _create_sympy_array_function( + self, + name: str, + sig: Optional[Signature] = None, + num_dims: Optional[list[int]] = None, + is_call: Optional[bool] = False) -> sympy.Function: '''Creates a Function class with the given name to be used for SymPy parsing. This Function overwrites the conversion to string, and will replace the triplicated array indices back to the normal Fortran @@ -271,7 +272,7 @@ def _create_sympy_array_function(self, @staticmethod def _ndims_for_struct_access(sig: Signature, - sva: SingleVariableAccessInfo) -> List[int]: + sva: SingleVariableAccessInfo) -> list[int]: ''' The same Signature can be accessed with different numbers of indices, e.g. a%b, a%b(1) and a(1)%b. This routine examines all accesses and @@ -333,8 +334,8 @@ def _specialise_array_symbol(sym: Symbol, sva: SingleVariableAccessInfo): return # ------------------------------------------------------------------------- - def _create_type_map(self, list_of_expressions: List[Node], - identical_variables: Optional[Dict[str, str]] = None, + def _create_type_map(self, list_of_expressions: Iterable[Node], + identical_variables: Optional[dict[str, str]] = None, all_variables_positive: Optional[bool] = None): '''This function creates a dictionary mapping each access in any of the expressions to either a SymPy Function (if the reference @@ -390,6 +391,7 @@ def _create_type_map(self, list_of_expressions: List[Node], # conversion). First, add all reserved names so that these names will # automatically be renamed. The symbol table is used later to also # create guaranteed unique names for lower and upper bounds. + # pylint: disable=too-many-locals, too-many-branches self._symbol_table = SymbolTable() for reserved in SymPyWriter._RESERVED_NAMES: self._symbol_table.new_symbol(reserved) @@ -502,11 +504,9 @@ def no_bounds(self) -> str: # ------------------------------------------------------------------------- @property - def type_map(self) -> Dict[str, Union[sympy.core.symbol.Symbol, + def type_map(self) -> dict[str, Union[sympy.core.symbol.Symbol, sympy.core.function.Function]]: ''':returns: the mapping of names to SymPy symbols or functions. - :rtype: Dict[str, Union[:py:class:`sympy.core.symbol.Symbol`, - :py:class:`sympy.core.function.Function`]] ''' return self._sympy_type_map @@ -515,9 +515,9 @@ def type_map(self) -> Dict[str, Union[sympy.core.symbol.Symbol, def _to_str( self, list_of_expressions: Union[Node, Iterable[Node]], - identical_variables: Optional[Dict[str, str]] = None, + identical_variables: Optional[dict[str, str]] = None, all_variables_positive: Optional[bool] = False) -> Union[str, - List[str]]: + list[str]]: '''Converts PSyIR expressions to strings. It will replace Fortran- specific expressions with code that can be parsed by SymPy. The argument can either be a single element (in which case a single string @@ -538,14 +538,11 @@ def _to_str( :returns: the converted strings(s). ''' - is_list = isinstance(list_of_expressions, (Iterable)) - if not is_list: - # Make mypy happy: - assert isinstance(list_of_expressions, Node) + is_list = True + if isinstance(list_of_expressions, Node): + is_list = False list_of_expressions = [list_of_expressions] - # Make mypy happy: - assert isinstance(list_of_expressions, Iterable) # Create the type map in `self._sympy_type_map`, which is required # when converting these strings to SymPy expressions self._create_type_map(list_of_expressions, @@ -565,11 +562,11 @@ def _to_str( # ------------------------------------------------------------------------- def __call__( self, - list_of_expressions: Union[Node, List[Node]], - identical_variables: Optional[Dict[str, str]] = None, + list_of_expressions: Union[Node, list[Node]], + identical_variables: Optional[dict[str, str]] = None, all_variables_positive: Optional[bool] = False) \ -> Union[sympy.core.basic.Basic, - List[sympy.core.basic.Basic]]: + list[sympy.core.basic.Basic]]: ''' This function takes a list of PSyIR expressions, and converts them all into Sympy expressions using the SymPy parser. @@ -609,9 +606,9 @@ def __call__( raise TypeError("Dictionary identical_variables " "contains a non-string key or value.") - is_list = isinstance(list_of_expressions, (tuple, list)) - if not is_list: - assert isinstance(list_of_expressions, Node) + is_list = True + if isinstance(list_of_expressions, Node): + is_list = False list_of_expressions = [list_of_expressions] expression_str_list = self._to_str( @@ -676,7 +673,7 @@ def arrayofstructuresreference_node( sig, indices = node.get_signature_and_indices() all_dims = [] - for i, name in enumerate(sig): + for i, _ in enumerate(sig): if indices[i]: for index in indices[i]: all_dims.append(index) @@ -842,7 +839,7 @@ def binaryoperation_node(self, node: BinaryOperation) -> str: # ------------------------------------------------------------------------ def gen_indices(self, - indices: List[Node], + indices: Iterable[Node], var_name: Optional[str] = None): '''Given a list of PSyIR nodes representing the dimensions of an array, return a list of strings representing those array dimensions. diff --git a/src/psyclone/psyir/frontend/sympy_reader.py b/src/psyclone/psyir/frontend/sympy_reader.py index 3fce343137..f76c42c841 100644 --- a/src/psyclone/psyir/frontend/sympy_reader.py +++ b/src/psyclone/psyir/frontend/sympy_reader.py @@ -38,18 +38,18 @@ ''' from sympy.printing.printer import Printer +from sympy.printing.str import StrPrinter from psyclone.psyir.frontend.fortran import FortranReader # pylint: disable=invalid-name class FortranPrinter(Printer): - '''A helper class that converts Fortran logical operators back - to Fortran format (while SymPy has a Fortran printer (fcode) does this - as well, it does not handle e.g. Fortran Array expressions (a(2:5)), - so we need to use the not-Fortran-aware output to a normal string, - but handle logical operators separately. - ''' + '''Specialise the SymPy Printer to convert logical operators back to + Fortran format. While SymPy has a Fortran printer (fcode), it does not + handle e.g. Fortran Array expressions (a(2:5)), so we specialise the + generic SymPy Printer and handle the necessary conversions.''' + def _print_And(self, expr): '''Called when converting an AND expression.''' return f"({'.AND.' .join(self._print(i) for i in expr.args)})" From 5d614bf451a2a746594d076b2861ba338879224c Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Sat, 5 Jul 2025 11:51:58 +1000 Subject: [PATCH 8/9] #2923 Fixing flake8 issues. --- src/psyclone/psyir/frontend/sympy_reader.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/psyclone/psyir/frontend/sympy_reader.py b/src/psyclone/psyir/frontend/sympy_reader.py index f76c42c841..db8e3f3198 100644 --- a/src/psyclone/psyir/frontend/sympy_reader.py +++ b/src/psyclone/psyir/frontend/sympy_reader.py @@ -38,7 +38,6 @@ ''' from sympy.printing.printer import Printer -from sympy.printing.str import StrPrinter from psyclone.psyir.frontend.fortran import FortranReader @@ -48,7 +47,7 @@ class FortranPrinter(Printer): '''Specialise the SymPy Printer to convert logical operators back to Fortran format. While SymPy has a Fortran printer (fcode), it does not handle e.g. Fortran Array expressions (a(2:5)), so we specialise the - generic SymPy Printer and handle the necessary conversions.''' + generic SymPy Printer and handle the necessary conversions.''' def _print_And(self, expr): '''Called when converting an AND expression.''' From 5f180f6b548460dd82ec4dbe0ae5f859af7b0315 Mon Sep 17 00:00:00 2001 From: Sergi Siso Date: Mon, 7 Jul 2025 11:56:43 +0100 Subject: [PATCH 9/9] #2923 Update changelog --- changelog | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/changelog b/changelog index 795583f6b8..6564e95063 100644 --- a/changelog +++ b/changelog @@ -1,4 +1,7 @@ - 46) PR 3033 for #3031 and #3032. Prevent inserting profiling callipers to + 47) PR #2997 for #223. Add support for logical operators in the SymPy + comparisons. + + 46) PR #3033 for #3031 and #3032. Prevent inserting profiling callipers to NEMO functions and any elemental function. Also remove pure attribute when inserting psy_data wrappers.