stfc / PSyclone

Domain-specific compiler and code transformation system for Finite Difference/Volume/Element Earth-system models in Fortran
BSD 3-Clause "New" or "Revised" License
106 stars 28 forks source link

Update LFRic transformation signature? #2739

Open sergisiso opened 3 weeks ago

sergisiso commented 3 weeks ago

Since we are talking about 3.0, one aspect that I wanted to discuss is if we should also update the LFRic scripts signature in this major release to use the root psyir node as scprit input argument (like in the generic transformation scripts).

Instead of:

def trans(psy):
    '''
    :param psy: the PSy object that PSyclone has constructed.
    :type psy: :py:class:`psyclone.dynamo0p3.DynamoPSy`
    '''

    # Loop over all the Invokes in the PSy object
    for invoke in psy.invokes.invoke_list:
        print("Transforming invoke '"+invoke.name+"'...")
        schedule = invoke.schedule

it would be:

def trans(psyir):
    '''
    :param psyir: the PSyIR of the generated PSy-layer
    :type psyir: :py:class:`psyclone.psyir.nodes.FileContainer`
    ''''
    # Loop over all the Invokes Subroutines
    for schedule in psyir.walk(InvokeSchedule):
        print("Transforming invoke " + schedule.name)

The main advantage is that it hides the PSy->Invokes->Invoke->InvokeSchedule structure:

Other minor advantages are:

What do @arporter @hiker @christophermaynard @TeranIvy think?

arporter commented 3 weeks ago

Thanks @sergisiso, I like this proposal a lot. Presumably, if we were dealing with a named invoke (i.e. a call invoke(name="big-science", kernel1(x,y))) then that name would still be available in the script as the name of the InvokeSchedule?

sergisiso commented 3 weeks ago

Yes, I didn't discuss it here but there are some scripts that use psy.invokes.get("invoke_0") which would then be a schedule.name == "invoke_0" check. Some print psy.name which will then be psyir.name. And since we would be touching all these loops we could also rename InvokeSchedule to InvokeRoutine (since it inherits from Routine) and it will be even clearer to do: for subroutine in psyir.walk(InvokeRoutine): (since Schedule is a very specifc PSyclone concept but Routine is a broader Fortran concept).

hiker commented 3 weeks ago

Yes, please - change that, I always hated the exposure of the PSy structure.

For a while, maybe the FileContainer could provide a compatibility layer?

    for invoke_name in psy.invokes.names:
        invoke = psy.invokes.get(invoke_name)

That would avoid the effort of having to all scripts when the next release comes out? These function could print a deprecated functionality warning or so.

sergisiso commented 3 weeks ago

Ok, I will start implementing this