Skip to content

Commit

Permalink
stfc#2070 Fix PR remarks and pass pycodestyle
Browse files Browse the repository at this point in the history
  • Loading branch information
svalat committed Sep 11, 2023
1 parent 303b105 commit 23d84dd
Show file tree
Hide file tree
Showing 8 changed files with 246 additions and 108 deletions.
2 changes: 1 addition & 1 deletion src/psyclone/domain/lfric/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,4 @@
'LFRicExtractDriverCreator',
'LFRicInvoke',
'LFRicLoopBounds',
'LFRicSymbolTable']
'LFRicSymbolTable']
7 changes: 3 additions & 4 deletions src/psyclone/domain/lfric/lfric_loop_bounds.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
# Modified J. Henrichs, Bureau of Meteorology
# Modified A. B. G. Chalk and N. Nobre, STFC Daresbury Lab

''' This module provides the LFRicLoopBounds Class that handles all variables
''' This module provides the LFRicLoopBounds Class that handles all variables
required for specifying loop limits within an LFRic PSy-layer routine.'''

# Imports
Expand All @@ -46,9 +46,8 @@


class LFRicLoopBounds(LFRicCollection):

'''
Handles all variables required for specifying loop limits within
'''
Handles all variables required for specifying loop limits within
an LFRic PSy-layer routine.
'''

Expand Down
5 changes: 4 additions & 1 deletion src/psyclone/f2pygen.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,10 @@ class ACCDirective(Directive):
'loop').
'''
def __init__(self, root, line, position, dir_type):
self._types = ["parallel", "kernels", "enter data", "loop", "routine", "wait"]
self._types = [
"parallel", "kernels", "enter data", "loop", "routine",
"wait"
]
self._positions = ["begin", "end"]

super(ACCDirective, self).__init__(root, line, position, dir_type)
Expand Down
101 changes: 55 additions & 46 deletions src/psyclone/psyir/nodes/acc_directives.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
from psyclone.psyir.nodes.acc_mixins import ACCAsyncMixin
from psyclone.core.signature import Signature


class ACCDirective(metaclass=abc.ABCMeta):
# pylint: disable=too-few-public-methods
'''
Expand Down Expand Up @@ -164,6 +165,7 @@ def begin_string(self):
'''
return "acc routine"


class ACCEnterDataDirective(ACCStandaloneDirective, ACCAsyncMixin):
'''
Class representing a "!$ACC enter data" OpenACC directive in
Expand All @@ -176,9 +178,10 @@ class ACCEnterDataDirective(ACCStandaloneDirective, ACCAsyncMixin):
directive as a child.
:type parent: :py:class:`psyclone.psyir.nodes.Node`
:param async_queue: Enable async support and attach it to the given queue.
Can use False to disable, True to enable on default stream.
Int to attach to the given stream ID or use a variable Signature
to say at runtime what stream to be used.
Can use False to disable, True to enable on default
stream. Int to attach to the given stream ID or use a
variable Signature to say at runtime what stream to be
used.
:type async_queue: bool | int | :py:class: psyclone.core.Signature
'''
def __init__(self, children=None, parent=None, async_queue=False):
Expand Down Expand Up @@ -291,21 +294,18 @@ class ACCParallelDirective(ACCRegionDirective, ACCAsyncMixin):
in the PSyIR. By default it includes the 'DEFAULT(PRESENT)' clause which
means this node must either come after an EnterDataDirective or within
a DataDirective.
'''
def __init__(self, children=None, parent=None, async_queue=False):
'''
Constructor of the class.
:param children: the PSyIR nodes to be enclosed in the Kernels region \
:param children: the PSyIR nodes to be enclosed in the ACC parallel region\
and which are therefore children of this node.
:type children: List[:py:class:`psyclone.psyir.nodes.Node`]
:param parent: the parent of this node in the PSyIR.
:type parent: sub-class of :py:class:`psyclone.psyir.nodes.Node`
:param async_queue: Make the directive asynchonous and attached to the given
stream identified by an ID or by a variable name pointing to
an integer.
:type async_queue: bool | :py:class:psyclone.psyir.nodes.Reference | ints
'''
:param async_queue: Make the directive asynchonous and attached to the
given stream identified by an ID or by a variable
name pointing to an integer.
:type async_queue: bool|:py:class:`psyclone.psyir.nodes.Reference`|int
'''
def __init__(self, children=None, parent=None, async_queue=False):
super().__init__(children=children, parent=parent)
ACCAsyncMixin.__init__(self, async_queue)

Expand Down Expand Up @@ -335,7 +335,8 @@ def gen_code(self, parent):
'''
self.validate_global_constraints()

# remove the "acc parallel" added by begin_string() and keep only the parameters
# remove the "acc parallel" added by begin_string() and keep only the
# parameters
begin_args = ' '.join(self.begin_string().split()[2:])

# add the directive
Expand Down Expand Up @@ -618,27 +619,25 @@ def end_string(self):
class ACCKernelsDirective(ACCRegionDirective, ACCAsyncMixin):
'''
Class representing the !$ACC KERNELS directive in the PSyIR.
'''

def __init__(self, children=None, parent=None, default_present=True, async_queue=False):
'''
Constructor of the class.
:param children: the PSyIR nodes to be enclosed in the Kernels region \
:param children: the PSyIR nodes to be enclosed in the Kernels region \
and which are therefore children of this node.
:type children: List[:py:class:`psyclone.psyir.nodes.Node`]
:param parent: the parent of this node in the PSyIR.
:type parent: sub-class of :py:class:`psyclone.psyir.nodes.Node`
:param bool default_present: whether or not to add the "default(present)" \
clause to the kernels directive.
:param async_queue: Make the directive asynchonous and attached to the given
stream identified by an ID or by a variable name pointing to
an integer.
:type async_queue: bool | :py:class:psyclone.psyir.nodes.Reference | int
:raises NotImplementedError: if default_present is False.
'''
:type children: List[:py:class:`psyclone.psyir.nodes.Node`]
:param parent: the parent of this node in the PSyIR.
:type parent: sub-class of :py:class:`psyclone.psyir.nodes.Node`
:param bool default_present: whether or not to add the \
"default(present)" clause to the kernels \
directive.
:param async_queue: Make the directive asynchonous and attached to the
given stream identified by an ID or by a variable
name pointing to an integer.
:type async_queue: bool|:py:class:`psyclone.psyir.nodes.Reference`|int
:raises NotImplementedError: if default_present is False.
'''

def __init__(self, children=None, parent=None, default_present=True,
async_queue=False):
super().__init__(children=children, parent=parent)
ACCAsyncMixin.__init__(self, async_queue)
self._default_present = default_present
Expand Down Expand Up @@ -866,10 +865,12 @@ class ACCUpdateDirective(ACCStandaloneDirective, ACCAsyncMixin):
clause on the update directive (this instructs the
directive to silently ignore any variables that are not
on the device).
:param async_queue: Make the directive asynchonous and attached to the given
stream identified by an ID or by a variable name pointing to
an integer.
:type async_queue: Optional[bool|:py:class:psyclone.psyir.nodes.Reference|int]
:param async_queue: Make the directive asynchonous and attached to the \
given stream identified by an ID or by a variable name
pointing to an integer.
:type async_queue: Optional[
bool|:py:class:`psyclone.psyir.nodes.Reference`|int
]
:type if_present: Optional[bool]
'''

Expand Down Expand Up @@ -999,7 +1000,8 @@ def begin_string(self):
# async
asyncvalue = self._build_async_string()

return f"acc update {condition}{self._direction}({sym_list}){asyncvalue}"
return \
f"acc update {condition}{self._direction}({sym_list}){asyncvalue}"


def _sig_set_to_string(sig_set):
Expand All @@ -1023,7 +1025,7 @@ class ACCWaitDirective(ACCStandaloneDirective):
Class representing the !$ACC WAIT directive in the PSyIR.
:param wait_queue: Which ACC async stream to wait. None to wait all.
:type wait_queue: Optional[:py:class:psyclone.psyir.nodes.Reference, int]
:type wait_queue: Optional[:py:class:`psyclone.psyir.nodes.Reference`, int]
'''
def __init__(self, wait_queue=None):
# call parent
Expand All @@ -1045,22 +1047,27 @@ def __eq__(self, other):
def wait_queue(self):
'''
:returns: The queue to wait on.
:rtype: Optional[int, :py:class:psyclone.psyir.nodes.Reference]
:rtype: Optional[int, :py:class:`psyclone.psyir.nodes.Reference`]
'''
return self._wait_queue

@wait_queue.setter
def wait_queue(self, wait_queue):
'''
Setter to assign a specific wait queue to wait for.
:param wait_queue: The wait queue to expect, or None for all.
:type wait_queue: Optional[int, :py:class:psyclone.psyir.nodes.Reference]
:type wait_queue: Optional[int, \
:py:class:`psyclone.psyir.nodes.Reference`]
:raises TypeError: if `wait_queue` is of the wrong type
'''
# check
if wait_queue != None and not isinstance(wait_queue, (int, Reference)):
raise TypeError("Invalid value type as wait_group, shoule be in (None, int, Signature) !")

if (wait_queue is not None
and not isinstance(wait_queue, (int, Reference))):
raise TypeError("Invalid value type as wait_group, shoule be"
"in (None, int, Signature) !")

# set
self._wait_queue = wait_queue

Expand All @@ -1072,7 +1079,8 @@ def gen_code(self, parent):
content.
:type parent: sub-class of :py:class:`psyclone.f2pygen.BaseGen`
'''
# remove the "acc wait" added by begin_string() and keep only the parameters
# remove the "acc wait" added by begin_string() and keep only the
# parameters
args = ' '.join(self.begin_string().split()[2:])

# Generate the directive
Expand All @@ -1091,7 +1099,7 @@ def begin_string(self):
result = "acc wait"

# handle specifying groups
if self._wait_queue != None:
if self._wait_queue is not None:
if isinstance(self._wait_queue, Reference):
result += f" ({self._wait_queue.name})"
else:
Expand All @@ -1100,6 +1108,7 @@ def begin_string(self):
# ok return it
return result


# For automatic API documentation generation
__all__ = ["ACCRegionDirective", "ACCEnterDataDirective",
"ACCParallelDirective", "ACCLoopDirective", "ACCKernelsDirective",
Expand Down
60 changes: 36 additions & 24 deletions src/psyclone/psyir/nodes/acc_mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,51 +41,61 @@

from psyclone.psyir.nodes.reference import Reference


class ACCAsyncMixin(metaclass=abc.ABCMeta):
'''
Class handling the common code to handle the async keyword on related acc
directives.
:param async_queue: Enable async support and attach it to the given queue.
Can use False to disable, True to enable on default stream.
Int to attach to the given stream ID or use a variable Reference
to say at runtime what stream to be used.
:type async_queue: bool | int | :py:class: psyclone.core.Reference
Can use False to disable, True to enable on default
stream. Use int to attach to the given stream ID or
use a variable Reference to say at runtime what stream
to be used.
:type async_queue: bool | int | :py:class:`psyclone.core.Reference`
'''
def __init__(self, async_queue=False):
self.async_queue = async_queue

@property
def async_queue(self):
'''
:returns: whether or not async is enabled and if so, which queue this node
is associated with. (True indicates the default stream.)
:returns: whether or not async is enabled and if so, which queue this
node is associated with. (True indicates the default stream.)
Can use False to disable, True to enable on default stream.
Int to attach to the given stream ID or use a variable Reference
to say at runtime what stream to be used.
:rtype async_queue: bool | int | :py:class: psyclone.core.Reference
Int to attach to the given stream ID or use a variable
Reference to say at runtime what stream to be used.
:rtype async_queue: bool | int | :py:class:`psyclone.core.Reference`
'''
return self._async_queue

@async_queue.setter
def async_queue(self, async_queue):
'''
:param async_queue: Enable async support and attach it to the given queue.
Can use False to disable, True to enable on default stream.
Int to attach to the given stream ID or use a variable Reference
to say at runtime what stream to be used.
:type async_queue: bool | int | :py:class: psyclone.core.Reference
:param async_queue: Enable async support and attach it to the given
queue. Can use False to disable, True to enable on
default stream. Int to attach to the given stream
ID or use a variable Reference to say at runtime
what stream to be used.
:type async_queue: bool | int | :py:class:`psyclone.core.Reference`
:raises TypeError: if `wait_queue` is of the wrong type
'''
# check
if async_queue != None and not isinstance(async_queue, (bool, Reference, int)):
raise TypeError(f"Invalid async_queue value, expect Reference or integer or None or False, got : {async_queue}")

if (async_queue is not None and
not isinstance(async_queue, (bool, Reference, int))):
raise TypeError(f"Invalid async_queue value, expect Reference or "
f"integer or None or bool, got : {async_queue}")

# assign
self._async_queue = async_queue

def _build_async_string(self):
'''
Build the async arg to concat to the acc directive when generating the code.
Build the async arg to concat to the acc directive when generating the
code.
:returns: The `async()` option to add to the directive.
:rtype: str
'''
# default
result = ""
Expand All @@ -97,11 +107,13 @@ def _build_async_string(self):
elif isinstance(self.async_queue, int):
result = f" async({self.async_queue})"
elif isinstance(self.async_queue, Reference):
result = f" async({self.async_queue.name})"

from psyclone.psyir.backend.fortran import FortranWriter
text = FortranWriter()(self.async_queue)
result = f" async({text})"

# ok
return result

def __eq__(self, other):
'''
Checks whether two nodes are equal. Two ACCAsyncMixin are
Expand All @@ -112,7 +124,7 @@ def __eq__(self, other):
:returns: whether other is equal to self.
:rtype: bool
'''
if type(self) != type(other):
if type(self) is not type(other):
return False
else:
return self.async_queue == other.async_queue
return self.async_queue == other.async_queue
Loading

0 comments on commit 23d84dd

Please sign in to comment.