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

(Closes #2499) scalarization transformation implementation #2563

Merged
merged 74 commits into from
Feb 27, 2025
Merged
Show file tree
Hide file tree
Changes from 72 commits
Commits
Show all changes
74 commits
Select commit Hold shift + click to select a range
59d9875
First code towards #2499
LonelyCat124 Apr 29, 2024
ede95c7
forgotten file
LonelyCat124 Apr 29, 2024
6ae3ae3
First implementation of scalarization_trans and unit tests for the pr…
LonelyCat124 Apr 30, 2024
c7cf88c
Linting errors
LonelyCat124 Apr 30, 2024
577d702
Linting errors
LonelyCat124 Apr 30, 2024
d76bfb6
Final commits ready for a review
LonelyCat124 May 1, 2024
2523a6b
Added compile test
LonelyCat124 May 1, 2024
fece2bd
linting
LonelyCat124 May 1, 2024
ed75417
Changes to use filter to make the code easier to understand
LonelyCat124 May 20, 2024
8504137
Merged master
LonelyCat124 Jan 9, 2025
bd2144b
Changes to work with master
LonelyCat124 Jan 9, 2025
047c4a6
First section of rewriting, coverage issues to fix
LonelyCat124 Jan 10, 2025
b1d9544
linting error
LonelyCat124 Jan 10, 2025
2988358
Merge branch 'master' into 2499_scalarization_trans
LonelyCat124 Jan 10, 2025
dc69373
Updated tests for coverage. Now need to do some more complex function…
LonelyCat124 Jan 10, 2025
1a1531a
Missing coverage for other types of read
LonelyCat124 Jan 10, 2025
a506244
linting fix
LonelyCat124 Jan 10, 2025
75b77d7
Added another test
LonelyCat124 Jan 10, 2025
280cdb6
Added documentation to scalarization trans including an example
LonelyCat124 Jan 13, 2025
c97975f
Merge branch 'master' into 2499_scalarization_trans
LonelyCat124 Jan 13, 2025
74c7081
Fixed doc errors
LonelyCat124 Jan 14, 2025
058b3ef
Changes for the review
LonelyCat124 Jan 22, 2025
fae6466
Merge branch 'master' into 2499_scalarization_trans
LonelyCat124 Jan 22, 2025
4984dcd
Fixed the docuemntation
LonelyCat124 Jan 22, 2025
caf0176
Fixed linting
LonelyCat124 Jan 22, 2025
c4b9641
Changes towards review, blocked by #2870
LonelyCat124 Jan 23, 2025
e6c3072
Remove failing test that didn't meet the Fortran standard
LonelyCat124 Jan 23, 2025
ef3cd34
Added the transformation into the User guide
LonelyCat124 Jan 23, 2025
6d8bff5
Scalarize every loop to ensure nothing breaks in NEMO - Revert this l…
LonelyCat124 Jan 25, 2025
4974a6e
Fix tests to compile
LonelyCat124 Jan 27, 2025
409b859
Linting fix
LonelyCat124 Jan 27, 2025
8d51936
Failure to import scalarizationtrans fix
LonelyCat124 Jan 27, 2025
2ce0efa
Merge branch 'master' into 2499_scalarization_trans
LonelyCat124 Jan 27, 2025
dbae47c
Fix a bug from NEMO4 integration tests where the transformation faile…
LonelyCat124 Jan 27, 2025
a897bf9
git pushMerge branch '2499_scalarization_trans' of github.com:stfc/PS…
LonelyCat124 Jan 27, 2025
a840c8b
Fix another NEMO4 failure case
LonelyCat124 Jan 28, 2025
01912bd
fixed the issue in definition use chains when we find empty scopes
LonelyCat124 Jan 28, 2025
9e75ce6
flake issues
LonelyCat124 Jan 28, 2025
0b37212
Testing scalarization in passthrough instead of on GPU
LonelyCat124 Jan 28, 2025
9818e4b
Go again
LonelyCat124 Jan 29, 2025
c795eb1
another change to test things as we need to skip some files for now w…
LonelyCat124 Jan 29, 2025
4093d66
run the passthrough code for nemo4
LonelyCat124 Jan 29, 2025
956c164
Merge branch 'master' into 2499_scalarization_trans
LonelyCat124 Jan 31, 2025
250c259
Fix known bugs scalarizing NEMO
LonelyCat124 Jan 31, 2025
51814b5
linting
LonelyCat124 Jan 31, 2025
94f99eb
Linting
LonelyCat124 Jan 31, 2025
88c1eb3
Failing test added to try to work out NEMO5 issues
LonelyCat124 Feb 4, 2025
453a56b
Fix inquiry behaviour for is_read
LonelyCat124 Feb 4, 2025
c17a22d
Linting [skip-ci]
LonelyCat124 Feb 4, 2025
a0e7f2b
Change to handle inquiry functions in definitionusechains
LonelyCat124 Feb 4, 2025
173093d
Linting
LonelyCat124 Feb 4, 2025
1300fcf
Linting
LonelyCat124 Feb 4, 2025
51fe018
Merge branch 'master' into 2499_scalarization_trans
LonelyCat124 Feb 17, 2025
728dd7b
Changes to add less_than to symbolic maths and to handle scalarizatio…
LonelyCat124 Feb 17, 2025
4d4fbc9
linting
LonelyCat124 Feb 17, 2025
02de511
Swapping non-3.8 compatible type hint
LonelyCat124 Feb 17, 2025
37d9284
Fixing coverage
LonelyCat124 Feb 17, 2025
9e7c9b8
linting
LonelyCat124 Feb 17, 2025
e02b914
Changes to fix coverage!
LonelyCat124 Feb 17, 2025
056f577
Test fix to ensure it tests the expected part of the code
LonelyCat124 Feb 18, 2025
b766dfb
linting
LonelyCat124 Feb 18, 2025
a16eac4
Merge branch 'master' into 2499_scalarization_trans
LonelyCat124 Feb 19, 2025
ba8c0c1
Merged master
LonelyCat124 Feb 20, 2025
a64b6ab
Merge branch '2499_scalarization_trans' of github.com:stfc/PSyclone i…
LonelyCat124 Feb 20, 2025
756e152
Moving scalarization test into the cpu transformation script
LonelyCat124 Feb 24, 2025
a6d2fae
linting error
LonelyCat124 Feb 24, 2025
bf3eb0b
Fix bug when there is a write inside a conditional statement
LonelyCat124 Feb 25, 2025
75b7ca6
Missed a file
LonelyCat124 Feb 25, 2025
80dba44
Cleaned up the code ready for review
LonelyCat124 Feb 26, 2025
5c0557f
Error in transform script
LonelyCat124 Feb 26, 2025
027d9c0
Merge branch 'master' into 2499_scalarization_trans
LonelyCat124 Feb 26, 2025
12835bf
Changes for review
LonelyCat124 Feb 27, 2025
c28fea6
#2499 Update changelog, increase version, and fix nemo passthrough
sergisiso Feb 27, 2025
612f264
#2499 Rename scalarization to scalarisation
sergisiso Feb 27, 2025
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
3 changes: 3 additions & 0 deletions .github/workflows/nemo_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ jobs:
module load perl/${PERL_VERSION}
make -j 4 passthrough
make -j 4 compile-passthrough
make run-passthrough |& tee output.txt
# Check for full numerical reproducibility with KGO results
diff <(make -s output-passthrough) KGOs/run.stat.nemo4.splitz12.nvhpc.10steps

# PSyclone, compile and run MetOffice NEMO with OpenMP for GPUs
- name: NEMO MetOffice OpenMP for GPU
Expand Down
7 changes: 7 additions & 0 deletions doc/user_guide/transformations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,13 @@ can be found in the API-specific sections).

####

.. autoclass:: psyclone.psyir.transformations.ScalarizationTrans
:members: apply
:noindex:

####


Algorithm-layer
---------------

Expand Down
4 changes: 2 additions & 2 deletions examples/nemo/scripts/omp_cpu_trans.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ def trans(psyir):
:type psyir: :py:class:`psyclone.psyir.nodes.FileContainer`

'''

# If the environemnt has ONLY_FILE defined, only process that one file and
# nothing else. This is useful for file-by-file exhaustive tests.
only_do_file = os.environ.get('ONLY_FILE', False)
Expand All @@ -96,7 +95,8 @@ def trans(psyir):
hoist_local_arrays=False,
convert_array_notation=True,
convert_range_loops=True,
hoist_expressions=False
hoist_expressions=False,
scalarize_loops=False
)

if psyir.name not in PARALLELISATION_ISSUES:
Expand Down
2 changes: 1 addition & 1 deletion examples/nemo/scripts/passthrough.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,5 @@
FILES_TO_SKIP = []


def trans(_):
def trans():
''' Don't do any changes. '''
15 changes: 14 additions & 1 deletion examples/nemo/scripts/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
from psyclone.psyir.transformations import (
ArrayAssignment2LoopsTrans, HoistLoopBoundExprTrans, HoistLocalArraysTrans,
HoistTrans, InlineTrans, Maxval2LoopTrans, ProfileTrans,
Reference2ArrayRangeTrans)
Reference2ArrayRangeTrans, ScalarizationTrans)
from psyclone.transformations import TransformationError


Expand Down Expand Up @@ -283,6 +283,7 @@ def normalise_loops(
loopify_array_intrinsics: bool = True,
convert_range_loops: bool = True,
hoist_expressions: bool = True,
scalarize_loops: bool = False,
):
''' Normalise all loops in the given schedule so that they are in an
appropriate form for the Parallelisation transformations to analyse
Expand All @@ -299,6 +300,8 @@ def normalise_loops(
loops.
:param bool hoist_expressions: whether to hoist bounds and loop invariant
statements out of the loop nest.
:param scalarize_loops: whether to attempt to convert arrays to scalars
where possible, default is False.
'''
if hoist_local_arrays and schedule.name not in CONTAINS_STMT_FUNCTIONS:
# Apply the HoistLocalArraysTrans when possible, it cannot be applied
Expand Down Expand Up @@ -339,6 +342,16 @@ def normalise_loops(
except TransformationError:
pass

if scalarize_loops:
# Apply scalarization to every loop. Execute this in reverse order
# as sometimes we can scalarize earlier loops if following loops
# have already been scalarized.
loops = schedule.walk(Loop)
loops.reverse()
scalartrans = ScalarizationTrans()
for loop in loops:
scalartrans.apply(loop)

if hoist_expressions:
# First hoist all possible expressions
for loop in schedule.walk(Loop):
Expand Down
33 changes: 33 additions & 0 deletions src/psyclone/core/symbolic_maths.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,39 @@ def greater_than(exp1, exp2, all_variables_positive=None):
return SymbolicMaths.Fuzzy.TRUE
return SymbolicMaths.Fuzzy.FALSE

@staticmethod
def less_than(exp1, exp2, all_variables_positive=None):
'''
Determines whether exp1 is, or might be, numerically less than exp2.

:param exp1: the first expression for the comparison.
:type exp1: :py:class:`psyclone.psyir.nodes.Node`
:param exp1: the second expression for the comparison.
:type exp1: :py:class:`psyclone.psyir.nodes.Node`
:param Optional[bool] all_variables_positive: whether or not to assume
that all variables appearing in either expression are positive
definite. Default is not to make this assumption.

:returns: whether exp1 is, or might be, numerically less than exp2.
:rtype: :py:class:`psyclone.core.symbolic_maths.Fuzzy`

'''
diff_val = SymbolicMaths._subtract(
exp1, exp2,
all_variables_positive=all_variables_positive)
if isinstance(diff_val, core.numbers.Integer):
if diff_val.is_zero or diff_val.is_positive:
return SymbolicMaths.Fuzzy.FALSE
return SymbolicMaths.Fuzzy.TRUE

# We have some sort of symbolic result
result = diff_val.is_negative
if result is None:
return SymbolicMaths.Fuzzy.MAYBE
if result:
return SymbolicMaths.Fuzzy.TRUE
return SymbolicMaths.Fuzzy.FALSE

# -------------------------------------------------------------------------
@staticmethod
def solve_equal_for(exp1, exp2, symbol):
Expand Down
17 changes: 17 additions & 0 deletions src/psyclone/psyir/nodes/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -1782,6 +1782,23 @@ def update_parent_symbol_table(self, new_parent):

'''

def is_descendent_of(self, potential_ancestor) -> bool:
'''
Checks if this node is a descendant of the `potential_ancestor` node.

:param potential_ancestor: The Node to check whether its an ancestor
of self.
:type node: :py:class:`psyclone.psyir.nodes.Node`

:returns: whether potential_ancestor is an ancestor of this node.
'''
current_node = self
while (current_node is not potential_ancestor and
current_node.parent is not None):
current_node = current_node.parent

return current_node is potential_ancestor


# For automatic documentation generation
# TODO #913 the 'colored' routine shouldn't be in this module.
Expand Down
8 changes: 8 additions & 0 deletions src/psyclone/psyir/nodes/reference.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,18 @@ def is_read(self):
'''
# pylint: disable=import-outside-toplevel
from psyclone.psyir.nodes.assignment import Assignment
from psyclone.psyir.nodes.intrinsic_call import IntrinsicCall
parent = self.parent
if isinstance(parent, Assignment):
if parent.lhs is self:
return False

# If we have an intrinsic call parent then we need to check if its
# an inquiry. Inquiry functions don't read from their first argument.
if isinstance(parent, IntrinsicCall):
if parent.arguments[0] is self and parent.is_inquiry:
return False

# All references other than LHS of assignments represent a read. This
# can be improved in the future by looking at Call intents.
return True
Expand Down
74 changes: 50 additions & 24 deletions src/psyclone/psyir/tools/definition_use_chains.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,16 +250,20 @@ def find_forward_accesses(self):
.abs_position
+ 1
)
# We make a copy of the reference to have a detached
# node to avoid handling the special cases based on
# the parents of the reference.
chain = DefinitionUseChain(
self._reference.copy(),
body,
start_point=ancestor.abs_position,
stop_point=sub_stop_point,
)
chains.insert(0, chain)
# If we have a basic block with no children then skip it,
# e.g. for an if block with no code before the else
# statement.
if len(body) > 0:
# We make a copy of the reference to have a detached
# node to avoid handling the special cases based on
# the parents of the reference.
chain = DefinitionUseChain(
self._reference.copy(),
body,
start_point=ancestor.abs_position,
stop_point=sub_stop_point,
)
chains.insert(0, chain)
# If its a while loop, create a basic block for the while
# condition.
if isinstance(ancestor, WhileLoop):
Expand Down Expand Up @@ -300,6 +304,11 @@ def find_forward_accesses(self):
# Now add all the other standardly handled basic_blocks to the
# list of chains.
for block in basic_blocks:
# If we have a basic block with no children then skip it,
# e.g. for an if block with no code before the else
# statement.
if len(block) == 0:
continue
chain = DefinitionUseChain(
self._reference,
block,
Expand Down Expand Up @@ -449,6 +458,12 @@ def _compute_forward_uses(self, basic_block_list):
if defs_out is not None:
self._defsout.append(defs_out)
return
# If its parent is an inquiry function then its neither
# a read nor write if its the first argument.
if (isinstance(reference.parent, IntrinsicCall) and
reference.parent.is_inquiry and
reference.parent.arguments[0] is reference):
continue
if isinstance(reference, CodeBlock):
# CodeBlocks only find symbols, so we can only do as good
# as checking the symbol - this means we can get false
Expand Down Expand Up @@ -525,9 +540,7 @@ def _compute_forward_uses(self, basic_block_list):
if defs_out is None:
self._uses.append(reference)
elif reference.ancestor(Call):
# It has a Call ancestor so assume read/write access
# for now.
# We can do better for IntrinsicCalls realistically.
# Otherwise we assume read/write access for now.
if defs_out is not None:
self._killed.append(defs_out)
defs_out = reference
Expand Down Expand Up @@ -699,6 +712,12 @@ def _compute_backward_uses(self, basic_block_list):
abs_pos = reference.abs_position
if abs_pos < self._start_point or abs_pos >= stop_position:
continue
# If its parent is an inquiry function then its neither
# a read nor write if its the first argument.
if (isinstance(reference.parent, IntrinsicCall) and
reference.parent.is_inquiry and
reference.parent.arguments[0] is reference):
continue
if isinstance(reference, CodeBlock):
# CodeBlocks only find symbols, so we can only do as good
# as checking the symbol - this means we can get false
Expand Down Expand Up @@ -784,9 +803,7 @@ def _compute_backward_uses(self, basic_block_list):
if defs_out is None:
self._uses.append(reference)
elif reference.ancestor(Call):
# It has a Call ancestor so assume read/write access
# for now.
# We can do better for IntrinsicCalls realistically.
# Otherwise we assume read/write access for now.
if defs_out is not None:
self._killed.append(defs_out)
defs_out = reference
Expand Down Expand Up @@ -835,6 +852,11 @@ def find_backward_accesses(self):
# Now add all the other standardly handled basic_blocks to the
# list of chains.
for block in basic_blocks:
# If we have a basic block with no children then skip it,
# e.g. for an if block with no code before the else
# statement.
if len(block) == 0:
continue
chain = DefinitionUseChain(
self._reference,
block,
Expand Down Expand Up @@ -874,14 +896,18 @@ def find_backward_accesses(self):
).abs_position
else:
sub_start_point = self._reference.abs_position
chain = DefinitionUseChain(
self._reference.copy(),
body,
start_point=sub_start_point,
stop_point=sub_stop_point,
)
chains.append(chain)
control_flow_nodes.append(ancestor)
# If we have a basic block with no children then skip it,
# e.g. for an if block with no code before the else
# statement.
if len(body) > 0:
chain = DefinitionUseChain(
self._reference.copy(),
body,
start_point=sub_start_point,
stop_point=sub_stop_point,
)
chains.append(chain)
control_flow_nodes.append(ancestor)
# If its a while loop, create a basic block for the while
# condition.
if isinstance(ancestor, WhileLoop):
Expand Down
3 changes: 3 additions & 0 deletions src/psyclone/psyir/transformations/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@
ReplaceInductionVariablesTrans
from psyclone.psyir.transformations.reference2arrayrange_trans import \
Reference2ArrayRangeTrans
from psyclone.psyir.transformations.scalarization_trans import \
ScalarizationTrans


# For AutoAPI documentation generation
Expand Down Expand Up @@ -145,5 +147,6 @@
'Reference2ArrayRangeTrans',
'RegionTrans',
'ReplaceInductionVariablesTrans',
'ScalarizationTrans',
'TransformationError',
'ValueRangeCheckTrans']
Loading
Loading