PSyclone icon indicating copy to clipboard operation
PSyclone copied to clipboard

[LFRic] Change to make invoke 'calls' Fortran compliant

Open arporter opened this issue 2 years ago • 4 comments

Current implementation:

PSyclone is instructed to call one or more kernels through calls to a hypothetical "invoke" subroutine in algorithm source. These may be optionally named by adding a "name=" keyword argument.

This works well since fparser does not enforce the Fortran syntax requirement that no positional arguments may appear after a Keyword argument. Thus the "name=" argument may appear first, which is where it is generally wanted.

Problem:

If fparser were ever to more strictly interpret the Fortran syntax or an alternative parser (such as libflang) were to be used which did then our algorithm code would stop parsing.

Requirements

We would like to move the name to the invocation call name. Thus call invoke(name=<name>, ... becomes call invoke_<name>(....

The existing form of call invoke(... would represent an unnamed Invoke as currently seen when no "name=" argument is specified.

This has the advantage of being valid Fortran but also enforcing Fortran label syntax on the invocation names which is relevant to PSyclone's interests.

In order to ease migration to this new form we would like a release of PSyclone which support both this new form and the existing form with a "name" argument. Using both together would be an error.

After release we would deprecate the use of the "name" argument and the release after that would remove it.

arporter avatar Sep 19 '23 15:09 arporter

If fparser were ever to more strictly interpret the Fortran syntax or an alternative parser (such as libflang) were to be used which did then our algorithm code would stop parsing.

I agree, but wouldn't it be easier to move the name optional parameter to the end of the argument list? Or is the call invoke_<name> preferred?

sergisiso avatar Sep 19 '23 16:09 sergisiso

I think the preference is to have the name at (or somewhere near) the start.

arporter avatar Sep 20 '23 08:09 arporter

Assigning this to @mo-lottieturner as agreed with @oakleybrunt.

TeranIvy avatar Dec 19 '23 11:12 TeranIvy

We've had a discussion about this and are uneasy about the proposed solution: essentially, it changes the one special entry in the application namespace (invoke) to an infinite number of entries. We would therefore prefer to simply move the name= argument to the end of the argument list to the invoke.

arporter avatar Dec 19 '23 11:12 arporter