Rename the NEMO API?

[arporter]

@mnobre has suggested that we should rename the NEMO API. @hiker is in favour and suggested "EVOLUTION". We could go with "EVO" (as in Mitsubishi Evo which is fast :-)) or perhaps something else. What do people think?

[rupertford]

Renaming is a good idea but I don't like evo or evolution I'm afraid. How about "generic" and then we could have an additional -config for specific config files? Failing that, something that implies we are transforming existing code e.g. "transform".

[nmnobre]

Bold, and perhaps too bold... what about no name at all? Dynamo and GOcean would simply be treated as exceptions to the "default" mode of operation on generic codes. For instance, instead of:

if isinstance(sched, DynInvokeSchedule):
  from psyclone.dynamo0p3 import DynACCEnterDataDirective as AccEnterDataDir
elif isinstance(sched, GOInvokeSchedule):
  from psyclone.gocean1p0 import GOACCEnterDataDirective as AccEnterDataDir
elif isinstance(sched, NemoInvokeSchedule):
  from psyclone.nemo import NemoACCEnterDataDirective as AccEnterDataDir
else:
  raise InternalError()

we would now write:

from psyclone.psyir.nodes import ACCEnterDataDirective
if isinstance(sched, DynInvokeSchedule):
  from psyclone.dynamo0p3 import DynACCEnterDataDirective as ACCEnterDataDirective
elif isinstance(sched, GOInvokeSchedule):
  from psyclone.gocean1p0 import GOACCEnterDataDirective as ACCEnterDataDirective

[hiker]

Bold, and perhaps too bold... what about no name at all? Dynamo and GOcean would simply be treated as exceptions to the "default" mode of operation on generic codes. For instance, instead of:

I have to admit that I don't like that - PSyclone should (imho) focus on revolution, not making evolution the default.

...

from psyclone.psyir.nodes import ACCEnterDataDirective if isinstance(sched, DynInvokeSchedule): from psyclone.dynamo0p3 import DynACCEnterDataDirective as ACCEnterDataDirective elif isinstance(sched, GOInvokeSchedule): from psyclone.gocean1p0 import GOACCEnterDataDirective as ACCEnterDataDirective

I think there is an easier solution:

from psyclone.psyir.nodes import ACCEnterDataDirective
enter_data = AccEnterDataDirective.create()

The current API can be queried, so this is basically a kind of factory (I intend to propose the same for handling API specific transformations, since it's atm a big mess to know when to use a api-specific, and when a generic one).

[arporter]

@sergisiso, @rupertford and I discussed this earlier and came down in favour (I think) of having no name as there's no actual API. Therefore, psyclone without -api would just do code transformation. This would break the MO build system as currently we have dynamo0.3 as the default API and I think their script relies on that - i.e. it doesn't currently specify -api.

An alternative option we discussed was to have an entirely different script as entry point for the code-transformation path, e.g. psytrans instead of psyclone or similar.

The one thing we definitely concluded was that we need to take a look at those classes that have been specialised for 'NEMO' and see what functionality it is that they add.

[arporter]

[arporter]

NemoAllArrayAccess2LoopTrans just uses NemoArrayAccess2LoopTrans. This uses the information from the configuration file to name the loop variable that is introduced (i.e. according to which dimension of the array is being accessed).

NemoArrayRange2LoopTrans again uses config. information to name the loop variable. It also uses this to specify the type of NemoLoop that is created. It also uses the bound information from the config file to set the loop bounds (if the loop is over the full extent of a dimension). However, we know that this is error prone and we shouldn't do it! Finally, it also puts the body of the loop into a NemoKern if it consists purely of compute statements.

[arporter]

AFAICS, NemoKern (which subclasses InlinedKern) does not add any domain-specific functionality.

NemoLoop - sets the valid loop types (according to info. from the config. file) and implements a create which sets the loop type by looking at the name of the supplied loop variable.

[hiker]

@sergisiso, @rupertford and I discussed this earlier and came down in favour (I think) of having no name as there's no actual API.

One could argue that the coding style is the API - similar to PSyclone ignoring anything non-invoke, the GENERIC API ignores any loops not following the coding style (as defined in the config file). But, I know when to give up :)

Therefore, psyclone without -api would just do code transformation. This would break the MO build system as currently we have dynamo0.3 as the default API and I think their script relies on that - i.e. it doesn't currently specify -api.

I am not entirely sure about this: the PSyclone default API is specified in the config file. So, once my LFRic PR that adds a config file is merged, the config file will specify the default, so there would be no need (if we assume that we specify DEFAULTAPI=generic in the config file).

Note that the help message from PSyclone is incorrect (while it queries the config object for this, de-facto the config file has not been read at this time, since the config file to read can be specified on the command line sigh).

[arporter]

NemoPSy simply extends PSy with inline (which raises a NotImplementedError) and gen which uses the PSyIR Fortran backend.

NemoInvokeSchedule simply extends InvokeSchedule with the coded_kernels method that returns all InlinedKern objects in the schedule.

[arporter]

NemoInvokes is more problematic - its constructor is actually responsible for raising the PSyIR (https://psyclone-ref.readthedocs.io/en/latest/_static/html/nemo_8py_source.html#l00076). It probably shouldn't be! I think it was done this way so that the NEMO API appeared to work like the other APIs. NemoInvoke doesn't appear to bring anything to the party.

[arporter]

The only 'domain'-specific part of NemoLoopFuseTrans is that it requires that the names of the loop variables match. This seems an unnecessary restriction.

[arporter]

Finally, NemoConstants just contains the valid loop types read from the config. file.

So, in summary, the only distinguishing feature of the NEMO subclasses is their use of the map from loop variable to loop type and array dimension to loop variable name. NemoInvokes is more complicated because that is key to the way that PSyclone is able to handle existing code in the same way that it handles PSyKAl code.

In general, I think the idea that we have an homogeneous code base where we can reliably use loop variable names to infer domain-specific knowledge is an API (as @hiker said above). If we went truly general purpose then we wouldn't be able to make that assumption.

[rupertford]

I agree with this last paragraph. In practical terms, if we just use the (language-level) PSyIR and associated transformations then we are working with transformations on general purpose code, as we make no assumptions about the code structure. As soon as we infer any additional information from the code then we are no longer general purpose. Whether this is domain-specific information, as opposed to code-specific information I'm not sure - it's probably the latter. I guess the API for PSyclone is the structure of the configuration information.

[nmnobre]

In general, I think the idea that we have an homogeneous code base where we can reliably use loop variable names to infer domain-specific knowledge is an API (as @hiker said above). If we went truly general purpose then we wouldn't be able to make that assumption.

The way I see it, this "kind" of API can simply be extracted from a configuration file that dynamically changes the behaviour of PSyclone as needed. Can we not simply provide a list of variable names, their usual relative order in implicit loops, range and stride? Basically, we need only standardise the last few lines of psyclone.cfg so that everyone can build their own.

I mean, correct me if I'm wrong, but isn't this essentially what's already happening? I can edit the config file with another app's settings and run PSyclone in "NEMO mode" over that app and it should just work? I'm just asking for the naming to reflect this reality.

[arporter]

I think we're all starting to agree. We have concluded that there is such a thing as "NEMO mode" and it should have a name that isn't NEMO. We just need to agree what it should be called...

[nmnobre]

😝 Be bold. "NEMO mode" becomes "default mode" and people don't name defaults.

[hiker]

Am happy @rupertford 's 'generic' (or 'transform') - well, am probably happy with pretty much anything tbh, as long as it's not nemo :)

[sergisiso]

I vote for psyclone without "-api XXX" to default to generic code. I am undecided yet about what I prefer for the config file.

I am happy to take the

• NemoAllArrayAccess2LoopTrans,
• NemoAllArrayRange2LoopTrans,
• NemoArrayAccess2LoopTrans,
• NemoArrayRange2LoopTrans,
• NemoOuterArrayRange2LoopTrans,

conversion to generic transformations, since I need to fix them for the OpenMP CPU anyway.

But to make sure I understand the plan, which one of these is the idea:

1. move them to generic transformation and remove the 'smart' variable and bounds naming logic since it not always work.
2. move them to generic transformation and just comment out the 'smart' naming for now as this will be attempted from a generic config info in the future.
3. move them to generic transformation but keep the Nemo prefixed ones that inherit from the generic ones and leave there the 'smart' naming that will be attempted from a NEMO config info

[sergisiso]

As discussed, I now think the transformation was trying to do two things at the same time and the l/unbound->domain parameter can be a completely separate transformation that can also be applied to other user provided code. So I will do 1.

[sergisiso]

Turns out the naming is not the only API specific aspect of the transformations. There is also the fact that they create NemoLoop constructs and at the end of the transformation does.

if not assignment.lhs.walk(Range):
    # All valid array ranges have been replaced with explicit
    # loops. We now need to take the content of the loop and
    # place it within a NemoKern (inlined kernel) node.
    CreateNemoKernelTrans().apply(assignment.parent)

I didn't thought about this because I don't use this in the OpenMP transformation but I see in other parts of the code there are walks to these nodes types. Maybe I should move all but this into the generic transformation and then the NEMO specialization should 1) call the generic/super and 2) call the API raising on the resulting tree. Or should this left for the psyclone script? What do people think?

[arporter]

My feeling is that we should take these 'raising' aspects out of the transformation and have them separate.

[rupertford]

It is there because it is working on (raised) nemo-specific psyir, so we need to add an inlined kern to be consistent with the rest of the code. A generic language-level PSyIR version would obviously not have this. I would specialise the transformation for the nemo api as you suggest. Otherwise we would have to apply the transformation before raising.

[arporter]

We had another discussion about this today (which, happily, was consistent with a lot of what we've said above) and are coming down in favour of renaming "NEMO" to "PARAM" as it's all about parameters coming from the config. file. Other related words might be "mapping", "coding conventions", "rules", "homogeneous". If anyone can think of a fun name related to any of that then that would be great :-) e.g. rules->legal->LAW??

[sergisiso]

I am checking with everybody if we all agree that the NEMO API doesn't need a name and psyclone without an API will just do code transformation without any assumption other than what is expressed in the code and transformation script itself.

psyclone -s transformation.py -o output.f90 input.f90

This means that we have to drop the DEFAULTAPI config parameter, and if it doesn't have it already, the LFRic build system calls to psyclone will need to have the api specified:

psyclone -api dynamo0.3 ...

This may be a good time to also rename "dynamo0.3" to "lfric" and "gocean1.0" to "gocean". The api versioning has never been enforced, we do API backward incompatible changes all the time, which we synchronise with the psyclone versioning number and never update the api number.

Finally, I am wondering if API is still the best name, since other parts of LFRic will use psyclone but not necessarely the psykal route. It could be renamed DSL or PSYKAL-DSL, but this could be too big of a change and maybe should be a separate discussion:

psyclone -dsl lfric ... psyclone -psykal-dsl lfric ...

[nmnobre]

hear, hear!

[arporter]

I agree. I'm fairly relaxed about API though - that could be done separately if people care. The only thing that worries me is that this is a change that will impact the Met Office/LFRic but it may be that LFRic always invokes PSyclone with -api dynamo0.3 anyway @TeranIvy ? I suggest we keep dynamo0.3 as a synonym for lfric for backwards compatibility.

[hiker]

LFric provides the api parameter (both the old Makefile one and FAB as well ... I just realised, I need to checkout the latest version after the split, but I doubt that this was changed).

The changes to rename the api should be small, there is one lfric makefile (old style). FAB is actually a bit more of an issue, atm this is hard-coded in FAB itself - I will change that to make it configurable from the actual build script, it should only be a very small change (https://github.com/metomi/fab/issues/302)