Skip to content

Commit

Permalink
Merge pull request #2646 from stfc/2641_incorrect_module_names_in_psy…
Browse files Browse the repository at this point in the history 
…data

2641 incorrect module names in psydata
  • Loading branch information
@arporter
arporter authored Jul 9, 2024
2 parents ae4bc50 + 2e337f0 commit eeda0d0
Show file tree
Hide file tree
Showing 9 changed files with 135 additions and 123 deletions.
3 changes: 3 additions & 0 deletions changelog
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,9 @@
55) PR #2445 for #2444. Changes the kernel-extraction tooling so
that filenames are construction using dashes instead of colons.

56) PR #2646 for #2641. Fixes incorrect module names within the
PSyData API (e.g. when profiling).

release 2.5.0 14th of February 2024

1) PR #2199 for #2189. Fix bugs with missing maps in enter data
Expand Down
183 changes: 86 additions & 97 deletions doc/user_guide/profiling.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -462,14 +462,30 @@ The automatic name generation depends on whether you are using a
PSyKAl DSL or only the transformation capabilities of PSyclone. If
you are transforming existing code:

* the ``module_name`` string is set to the name of the parent
function/subroutine/program. This name is unique as Fortran requires
* the ``module_name`` string is set to the module which contains the
current code. If there is no module (e.g. a stand-alone subroutine),
the subroutine name is used instead. This name is unique as Fortran requires
these names to be unique within a program.

* the ``region_name`` is set to an ``r`` (standing for region) followed by
an integer which uniquely identifies the profile within the parent
function/subroutine/program (based on the profile node's position in
the PSyIR representation relative to any other profile nodes).
* the ``region_name`` is set to the name of the subroutine, followed by
an ``r`` (standing for region) followed by an integer which uniquely identifies
the profile within the parent function/subroutine/program (based on the
profile node's position in the PSyIR representation relative to any other
profile nodes). If there is no module name (which means the ``module_name``
is already set to the subroutine name), only the ``r`` followed by an
integer number is specified.

Example:
.. code-block::
:caption: Profiling names used when transforming existing code.
:emphasize-lines: 2,4
! If the subroutine tra_adv is contained in module tra_adv_mod:
CALL profile_psy_data % PreStart("tra_adv_mod", "tra_adv-r0", 0, 0)
! If the subroutinetra_adv is not contained in a module:
CALL profile_psy_data % PreStart("tra_adv", "r0", 0, 0)
For the :ref:`LFRic <lfric-api>` and
:ref:`GOcean <gocean-api>` APIs:
Expand All @@ -486,96 +502,69 @@ For the :ref:`LFRic <lfric-api>` and
node's position in the PSyIR representation relative to any other
profile nodes). For example:

.. code-block::
:caption: PSyIR with profiling nodes.
:emphasize-lines: 2
InvokeSchedule[invoke='invoke_0', dm=True]
0: Profile[]
Schedule[]
0: Profile[]
Schedule[]
0: HaloExchange[field='f2', type='region', depth=1,
check_dirty=True]
1: HaloExchange[field='m1', type='region', depth=1,
check_dirty=True]
2: HaloExchange[field='m2', type='region', depth=1,
check_dirty=True]
1: Profile[]
Schedule[]
0: Loop[type='', field_space='w1', it_space='cells',
upper_bound='cell_halo(1)']
Literal[value:'1', DataType.INTEGER]
Literal[value:'mesh%get_last_halo_cell(1)',
DataType.INTEGER]
Literal[value:'1', DataType.INTEGER]
Schedule[]
0: CodedKern testkern_code(a,f1,f2,m1,m2)
[module_inline=False]
1: Profile[]
Schedule[]
0: Loop[type='', field_space='w1',
it_space='cells',
upper_bound='cell_halo(1)']
Literal[value:'1', DataType.INTEGER]
Literal[value:'mesh%get_last_halo_cell(1)',
DataType.INTEGER]
Literal[value:'1', DataType.INTEGER]
Schedule[]
0: CodedKern testkern_code(a,f1,f2,m1,m2)
[module_inline=False]
2: Loop[type='', field_space='w1', it_space='cells',
upper_bound='cell_halo(1)']
Literal[value:'1', DataType.INTEGER]
Literal[value:'mesh%get_last_halo_cell(1)', DataType.INTEGER]
Literal[value:'1', DataType.INTEGER]
Schedule[]
0: CodedKern testkern_qr_code(f1,f2,m1,a,m2,istp)
[module_inline=False]
This is the code created for this example:

.. code-block::
:caption: Created Fortran source code with profiling regions.
:emphasize-lines: 5,6,7,18,19,24,25
MODULE container
CONTAINS
SUBROUTINE invoke_0(a, f1, f2, m1, m2, istp, qr)
...
CALL psy_data_3%PreStart("multi_functions_multi_invokes_psy", "invoke_0-r0", &
.. code-block::
:caption: PSyIR with profiling nodes.
:emphasize-lines: 2,4,12
InvokeSchedule[invoke='invoke_0', dm=True]
0: Profile[]
Schedule[]
0: Profile[]
Schedule[]
0: HaloExchange[field='f2', type='region', depth=1,
check_dirty=True]
1: HaloExchange[field='m1', type='region', depth=1,
check_dirty=True]
2: HaloExchange[field='m2', type='region', depth=1,
check_dirty=True]
1: Profile[]
Schedule[]
0: Loop[type='', field_space='w1', it_space='cells',
upper_bound='cell_halo(1)']
Literal[value:'1', DataType.INTEGER]
Literal[value:'mesh%get_last_halo_cell(1)',
DataType.INTEGER]
Literal[value:'1', DataType.INTEGER]
Schedule[]
0: CodedKern testkern_code(a,f1,f2,m1,m2)
[module_inline=False]
This is the code created for this example:

.. code-block::
:caption: Created Fortran source code with profiling regions.
:emphasize-lines: 5,6,7,17,18,19,24,30
MODULE container
CONTAINS
SUBROUTINE invoke_0(a, f1, f2, m1, m2, istp, qr)
...
CALL psy_data_2%PreStart("multi_functions_multi_invokes_psy", "invoke_0-r0", &
0, 0)
CALL psy_data%PreStart("multi_functions_multi_invokes_psy", "invoke_0-r1", 0, 0)
IF (f2_proxy%is_dirty(depth=1)) THEN
CALL f2_proxy%halo_exchange(depth=1)
END IF
IF (m1_proxy%is_dirty(depth=1)) THEN
CALL m1_proxy%halo_exchange(depth=1)
END IF
IF (m2_proxy%is_dirty(depth=1)) THEN
CALL m2_proxy%halo_exchange(depth=1)
END IF
CALL psy_data%PreEnd()
CALL psy_data_1%PreStart("multi_functions_multi_invokes_psy", "invoke_0-r2", &
CALL psy_data%PreStart("multi_functions_multi_invokes_psy", "invoke_0-r1", 0, 0)
IF (f2_proxy%is_dirty(depth=1)) THEN
CALL f2_proxy%halo_exchange(depth=1)
END IF
IF (m1_proxy%is_dirty(depth=1)) THEN
CALL m1_proxy%halo_exchange(depth=1)
END IF
IF (m2_proxy%is_dirty(depth=1)) THEN
CALL m2_proxy%halo_exchange(depth=1)
END IF
CALL psy_data%PreEnd()
CALL psy_data_1%PreStart("multi_functions_multi_invokes_psy", "invoke_0-r2", &
0, 0)
DO cell=1,mesh%get_last_halo_cell(1)
CALL testkern_code(...)
END DO
...
CALL psy_data_2%PreStart("multi_functions_multi_invokes_psy", &
"invoke_0-testkern_code-r3", 0, 0)
DO cell=1,mesh%get_last_halo_cell(1)
CALL testkern_code(...)
END DO
...
CALL psy_data_2%PostEnd()
CALL psy_data_1%PostEnd()
...
DO cell=1,mesh%get_last_halo_cell(1)
CALL testkern_qr_code(...)
END DO
...
CALL psy_data_3%PostEnd()
...
END SUBROUTINE invoke_0
END MODULE container
DO cell=1,mesh%get_last_halo_cell(1)
CALL testkern_code(...)
END DO
CALL psy_data_1%PostEnd()
...
DO cell=1,mesh%get_last_halo_cell(1)
CALL testkern_qr_code(...)
END DO
...
CALL psy_data_2%PostEnd()
...
END SUBROUTINE invoke_0
END MODULE container
8 changes: 4 additions & 4 deletions examples/gocean/eg5/profile/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,10 @@ library, you should see:
...
profile_PSyDataInit called
...
PreStart called for module 'invoke_0' region 'r0'
PostEnd called for module 'invoke_0' region 'r0'
PreStart called for module 'invoke_1_update_field' region 'r0'
PostEnd called for module 'invoke_1_update_field' region 'r0'
PreStart called for module 'psy_test' region 'invoke_0-r0'
PostEnd called for module 'psy_test' region 'invoke_0-r0'
PreStart called for module 'psy_test' region 'invoke_1_update_field-r0'
PostEnd called for module 'psy_test' region 'invoke_1_update_field-r0'
...
profile_PSyDataShutdown called
```
Expand Down
8 changes: 4 additions & 4 deletions examples/gocean/eg5/profile/test.x90
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@
!> stdout. Expected output:
!>
!> profile_PSyDataInit called
!> PreStart called for module 'psy_test' region 'invoke_0:r0'
!> PostEnd called for module 'psy_test' region 'invoke_0:r0'
!> PreStart called for module 'psy_test' region 'invoke_1_update_field:update_field_code:r0'
!> PostEnd called for module 'psy_test' region 'invoke_1_update_field:update_field_code:r0'
!> PreStart called for module 'psy_test' region 'invoke_0-r0'
!> PostEnd called for module 'psy_test' region 'invoke_0-r0'
!> PreStart called for module 'psy_test' region 'invoke_1_update_field-r0'
!> PostEnd called for module 'psy_test' region 'invoke_1_update_field-r0'

Program test
USE field_mod
Expand Down
Binary file modified psyclone.pdf
View file
Binary file not shown.
25 changes: 19 additions & 6 deletions src/psyclone/psyir/nodes/psy_data_node.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,12 @@
from psyclone.errors import InternalError, GenerationError
from psyclone.f2pygen import CallGen, TypeDeclGen, UseGen
from psyclone.psyir.nodes.codeblock import CodeBlock
from psyclone.psyir.nodes.routine import Routine
from psyclone.psyir.nodes.container import Container
from psyclone.psyir.nodes.file_container import FileContainer
from psyclone.psyir.nodes.node import Node
from psyclone.psyir.nodes.statement import Statement
from psyclone.psyir.nodes.routine import Routine
from psyclone.psyir.nodes.schedule import Schedule
from psyclone.psyir.nodes.statement import Statement
from psyclone.psyir.symbols import (SymbolTable, DataTypeSymbol, DataSymbol,
ContainerSymbol, UnresolvedType, Symbol,
UnsupportedFortranType, ImportInterface)
Expand Down Expand Up @@ -180,7 +182,6 @@ def __init__(self, ast=None, children=None, parent=None, options=None):
# TODO: #1394 Fix code duplication between
# PSyDataTrans and this PSyDataNode
name = options.get("region_name", None)

if name:
# pylint: disable=too-many-boolean-expressions
if not isinstance(name, tuple) or not len(name) == 2 or \
Expand Down Expand Up @@ -756,10 +757,16 @@ def gen_type_bound_call(typename, methodname, argument_list=None,
f"lowering but '{self}' is not.")

self.generate_symbols(routine_schedule.symbol_table)

module_name = self._module_name
if module_name is None:
module_name = routine_schedule.name
container = routine_schedule.ancestor(Container)
# If the current code is inside a module use the module name,
# otherwise (e.g. subroutine outside of any module) use the
# routine name as 'module_name'
if container and not isinstance(container, FileContainer):
module_name = container.name
else:
module_name = routine_schedule.name

if self._region_name:
region_name = self._region_name
Expand All @@ -774,7 +781,13 @@ def gen_type_bound_call(typename, methodname, argument_list=None,
if (isinstance(node, PSyDataNode) or
"psy-data-start" in node.annotations):
region_idx += 1
region_name = f"r{region_idx}"
# If the routine name is not used as 'module name' (in case of a
# subroutine outside of any modules), add the routine name
# to the region. Otherwise just use the number
if module_name != routine_schedule.name:
region_name = f"{routine_schedule.name}-r{region_idx}"
else:
region_name = f"r{region_idx}"

if not options:
options = {}
Expand Down
6 changes: 4 additions & 2 deletions src/psyclone/tests/psyir/nodes/profile_node_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,13 @@ def test_lower_to_lang_level_multi_node():
cblocks = sched.walk(CodeBlock)
ptree = cblocks[0].get_ast_nodes
code = str(ptree[0]).lower()
assert "call profile_psy_data % prestart(\"invoke_0\", \"r0\"" in code
assert ("call profile_psy_data % prestart(\"psy_single_invoke_two_"
"kernels\", \"invoke_0-r0\"" in code)
assert cblocks[0].annotations == ["psy-data-start"]
assert cblocks[1].annotations == []
ptree = cblocks[2].get_ast_nodes
code = str(ptree[0]).lower()
assert "call profile_psy_data_1 % prestart(\"invoke_0\", \"r1\"" in code
assert ("call profile_psy_data_1 % prestart(\"psy_single_invoke_two_"
"kernels\", \"invoke_0-r1\"" in code)
assert cblocks[2].annotations == ["psy-data-start"]
assert cblocks[3].annotations == []
6 changes: 4 additions & 2 deletions src/psyclone/tests/psyir/nodes/psy_data_node_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,8 @@ def test_psy_data_node_lower_to_language_level_with_options():
"post_var_list": [("", "b")]})

codeblocks = schedule.walk(CodeBlock)
expected = ['CALL psy_data % PreStart("invoke_0", "r0", 1, 1)',
expected = ['CALL psy_data % PreStart("psy_single_invoke_different_'
'iterates_over", "invoke_0-r0", 1, 1)',
'CALL psy_data % PreDeclareVariable("a", a)',
'CALL psy_data % PreDeclareVariable("b", b)',
'CALL psy_data % PreEndDeclaration',
Expand Down Expand Up @@ -509,7 +510,8 @@ def test_psy_data_node_lower_to_language_level_with_options():
"post_var_postfix": "_post"})

codeblocks = schedule.walk(CodeBlock)
expected = ['CALL psy_data % PreStart("invoke_0", "r0", 1, 1)',
expected = ['CALL psy_data % PreStart("psy_single_invoke_different_'
'iterates_over", "invoke_0-r0", 1, 1)',
'CALL psy_data % PreDeclareVariable("a_pre", a)',
'CALL psy_data % PreDeclareVariable("b_post", b)',
'CALL psy_data % PreEndDeclaration',
Expand Down
19 changes: 11 additions & 8 deletions tutorial/practicals/nemo/2_nemo_profiling/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ this session.)
see timing information printed to the terminal, e.g.:

===========================================
module::region count sum min average max
tra_adv::r0 1 0.718750000 0.718750000 0.718750000 0.718750000
module::region count sum min average max
tra_adv_mod::tra_adv-r0 1 0.718750000 0.718750000 0.718750000 0.718750000
===========================================

Timings are only reported for a single region because our mini-app consists
Expand All @@ -99,7 +99,7 @@ in this tutorial.

===========================================
module::region count sum min average max
tra_adv::r0 1 0.718750000 0.718750000 0.718750000 0.718750000
tra_adv_mod::tra_adv-r0 1 0.718750000 0.718750000 0.718750000 0.718750000
===========================================

If you examine the PSyIR that is displayed when running PSyclone with
Expand Down Expand Up @@ -168,15 +168,18 @@ transformation script to perform finer-grained profiling.
regions (r0-r13) reported by the timing library:

===========================================
module::region count sum min average max
tra_adv::r0 1 3.12500000E-02 3.12500000E-02 3.12500000E-02 3.12500000E-02
tra_adv::r1 1 0.00000000 0.00000000 0.00000000 0.00000000
tra_adv::r2 10 3.12500000E-02 0.00000000 3.12500005E-03 3.12500000E-02
module::region count sum min average max
tra_adv_mod::tra_adv-r0 1 2.24609375E-02 2.24609375E-02 2.24609375E-02 2.24609375E-02
tra_adv_mod::tra_adv-r1 1 0.00000000 0.00000000 0.00000000 0.00000000
tra_adv_mod::tra_adv-r2 10 5.66406250E-02 3.90625000E-03 5.66406269E-03 1.07421875E-02
tra_adv_mod::tra_adv-r3 10 5.17578125E-02 2.92968750E-03 5.17578144E-03 1.17187500E-02
...
tra_adv::r13 1 0.187500000 0.187500000 0.187500000 0.187500000
tra_adv_mod::tra_adv-r12 10 2.73437500E-02 9.76562500E-04 2.73437495E-03 3.90625000E-03
tra_adv_mod::tra_adv-r13 1 0.449218750 0.449218750 0.449218750 0.449218750
===========================================



2. Many PSyclone transformations allow additional options to be supplied
via a dictionary argument to the `apply()` method. The
profiling transformation, for instance, allows the user to supply a
Expand Down

0 comments on commit eeda0d0

Please sign in to comment.