(towards #1960 and #1799) Improve ArrayAssingment2Loop using call/routinesymbol datatype information by sergisiso · Pull Request #3343 · stfc/PSyclone

sergisiso · 2026-02-20T15:19:36Z

I started this branch to tackle the WHERE with reduction intrinsics, but as in my previous PRs I think this should be solved first in ArrayAssignment2Loop and then integrate it to the WHERE.

The reductions that end up with a scalar can are already be done because we know that that term does not need additional indexing after applying ArrayAssignment2Loop. However the call.datatype was quite limited. It worked for Intrinsics but not generic calls (even when we had a RoutineSymbol with information). And RoutineSymbols defaulted to NoType, even when that was not necessarely the case (I think UnresolvedType is a better default and use the frontend type, e.g. is a CallStmt, to give it the NoType).

I have made the changes needed to fix the above and I feel its already enough for a PR, the WHEREs will come in a follow up.

…end adds NoType when is part of a CallStmt

codecov · 2026-02-20T15:35:13Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.95%. Comparing base (df4a4f1) to head (ee70564).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3343   +/-   ##
=======================================
  Coverage   99.95%   99.95%           
=======================================
  Files         386      386           
  Lines       54187    54215   +28     
=======================================
+ Hits        54165    54193   +28     
  Misses         22       22

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…atatype_information

…ent2Loop

…atatype_information

sergisiso · 2026-02-24T14:37:46Z

@arporter @LonelyCat124 This is ready for review

arporter

Looks good Sergi - it's nice to see the improvements to the typing making their way 'upwards'. Just some tidying/clarification and I'm a bit puzzled by a couple of the tests.

arporter · 2026-02-25T13:29:17Z

src/psyclone/psyir/nodes/call.py

+        if self.routine:
+            if isinstance(self.routine.symbol, RoutineSymbol):
+                dt = self.routine.symbol.datatype
+                if not isinstance(dt, UnresolvedType):


What's the purpose of this if condition? Please could you add a comment.

arporter · 2026-02-25T14:32:53Z

src/psyclone/psyir/transformations/arrayassignment2loops_trans.py


+def _walk_until_non_elemental_call(
+        node: Node, search_type: type, stop_type: Optional[type] = None):
+    ''' Custom walk operation that stops at Calls that it are not


s/that it/that/

arporter · 2026-02-25T14:34:11Z

src/psyclone/psyir/transformations/arrayassignment2loops_trans.py



+def _walk_until_non_elemental_call(
+        node: Node, search_type: type, stop_type: Optional[type] = None):


Pls include the typehint for the return type.

arporter · 2026-02-25T14:36:44Z

src/psyclone/psyir/transformations/arrayassignment2loops_trans.py

+    if stop_type and isinstance(node, stop_type):
+        return local_list
+
+    if isinstance(node, Call) and not node.is_elemental:


Not for this PR, but, if we ever find ourselves needing another walk specialisation, perhaps we could add an optional stop_condition argument to walk that takes a routine that accepts a node as an argument and can do custom checks?

It crossed my mind but I thought it was too much for this PR, but I agree if we find the case again it may be worth it.

arporter · 2026-02-25T14:37:48Z

src/psyclone/tests/psyir/frontend/fparser2_call_handler_test.py

    assert isinstance(routine_symbol.interface, UnresolvedInterface)
    assert routine_symbol.name == "kernel"
    assert routine_symbol in call_node.scope.symbol_table.symbols
+    # This this is a "call ..." expression, the Routine is NoType (subroutine)


Ups, deleted

arporter · 2026-02-25T14:38:07Z

src/psyclone/tests/psyir/frontend/fparser2_call_handler_test.py

        assert isinstance(routine_symbol.interface, ImportInterface)
        assert routine_symbol.name == "kernel"
        assert routine_symbol in call_node.scope.symbol_table.symbols
+        # This this is a "call ..." expression, the Routine is NoType


arporter · 2026-02-25T14:44:22Z

src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py

+    assert errmsg in str(err.value)
    with pytest.raises(TransformationError) as err:
        trans.apply(assignments[3])
+    with pytest.raises(TransformationError) as err:


I should have asked for this originally but pls could you add a comment before the with ... saying which assignment is being tested.

arporter · 2026-02-25T14:52:49Z

src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py

+    # TODO #2429: This has the correct behaviour but for the wrong reason,
+    # once this issue is resolved the error message should be the expected
+    # one.
+    # assert errormsg in str(err.value)


I'm a bit confused here - you have this line commented out but then you have the same line at L876 apparently working?

The one in L876 checks the TransformationError after this one, the error message is different. The problem I am hitting here is that this returns
ArrayAssignment2LoopsTrans does not support array assignments that contain a CodeBlock anywhere in the expression because fparser considers the empty function calls as DerivedTypeConstructors and therefore codeblocks. I am planning to make these PSyIR calls soon, so I still want to test the right thing here, even if for now it is with a confusing error message.

I updated the comment

arporter · 2026-02-25T14:53:23Z

src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py

+        trans.validate(invalid[3])
    assert errormsg in str(err.value)

+    # There are impure (or we don't know the purity)


arporter · 2026-02-25T14:53:47Z

src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py

+    # TODO #2429: This has the correct behaviour but for the wrong reason,
+    # once this issue is resolved the error message should be the expected
+    # one.
+    # assert errormsg in str(err.value)


Same confusion here?

See response above

…ansformation and tests

sergisiso · 2026-02-26T09:18:56Z

@arporter This is ready for another review

sergisiso added 3 commits February 19, 2026 16:18

#1960 Add the call.datatype specialisation

8b2b4e3

#1960 Use call.datatype information to improve ArrayAssignment2Loop

77bf330

#1960 RoutineSymbol defaults to UnresolvedType, but the fparser front…

98e9bc1

…end adds NoType when is part of a CallStmt

sergisiso temporarily deployed to integration February 20, 2026 15:20 — with GitHub Actions Inactive

sergisiso added 5 commits February 24, 2026 09:24

Merge remote-tracking branch 'origin/master' into 1960_improve_call_d…

977aba0

…atatype_information

#1960 Improve support for calls that return an scalar in ArrayAssignm…

0c726bf

…ent2Loop

#1960 fix flake8

d139e2b

#1960 Fix typehints in ArrayAssignment2Loop

5f48578

Merge remote-tracking branch 'origin/master' into 1960_improve_call_d…

38bc83d

…atatype_information

sergisiso temporarily deployed to integration February 24, 2026 13:47 — with GitHub Actions Inactive

sergisiso requested review from LonelyCat124 and arporter February 24, 2026 14:37

sergisiso self-assigned this Feb 24, 2026

sergisiso added the ready for review label Feb 24, 2026

arporter added under review and removed ready for review labels Feb 25, 2026

arporter requested changes Feb 25, 2026

View reviewed changes

arporter added reviewed with actions and removed under review labels Feb 25, 2026

#1960 Improve comments and docstrings for the arrayassignment2loop tr…

f4302f8

…ansformation and tests

sergisiso added ready for review and removed reviewed with actions labels Feb 26, 2026

Merge branch 'master' into 1960_improve_call_datatype_information

ee70564

arporter added under review and removed ready for review labels Feb 27, 2026

sergisiso temporarily deployed to integration February 27, 2026 10:42 — with GitHub Actions Inactive



		def _walk_until_non_elemental_call(
		node: Node, search_type: type, stop_type: Optional[type] = None):

Conversation

sergisiso commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sergisiso commented Feb 24, 2026

Uh oh!

arporter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sergisiso commented Feb 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sergisiso commented Feb 20, 2026 •

edited

Loading

codecov bot commented Feb 20, 2026 •

edited

Loading