Psyclone 3 support #388
Psyclone 3 support #388
Conversation
3 better compiler support
… only one artefact set is involved when running PSyclone.
…les to the default build artefact collections.
…eady in the output directory.
…ot replacing artefact).
…n in build (not source).
…ive compiler names are longer (crayftn-cray, craycc-cray).
… to avoid confusion with Craycc.
… new stle arguments and vice versa.
…ms with compiler wrapper which do not support this).
source/fab/tools/psyclone.py
Outdated
| # The behaviour of PSyclone changes from 2.5.0 to the next | ||
| # release. But since head-of-trunk still reports 2.5.0, we | ||
| # need to run additional tests to see if we have the official | ||
| # 2.5.0 release, or current trunk (which already has the new | ||
| # command line options). PSyclone needs an existing file | ||
| # in order to work, so use __file__ to present this file. | ||
| # PSyclone will obviously abort since this is not a Fortran | ||
| # file, but we only need to check the error message to | ||
| # see if the domain name is incorrect (--> current trunk) | ||
| # or not (2.5.0 release) |
There was a problem hiding this comment.
Not only has PSyclone 2.5.0 been released but so has 3.0.0. So any special cases can be removed and this section can be streamlined.
There was a problem hiding this comment.
I was hoping to clean this up before it reaches you tbh. OK, long story:
ATM, PSyclone 3.0 supports backwards compatibility in the trans(psy) function (it now expects a proper psyir node), though a warning marks this as deprecated. I've asked if for the 3.1 release we could disable this backwards compatibility hack in 3.1 and properly fail, but the other devs pointed out that PSyclone 2.5 (with a psy argument) is still important, since it was just added to the NEMO build system.
At the same time I also noticed that standard environments for NG-ARCH (on Archer2 and Pawsey) still come with PSyclone 2.5 installed :( I have Scott (ngmo-environments dev) added to Pawsey, so he can port ngmo-environments to Crays, so hopefully we soon have an up-to-date one.
So, since my plot to just stop supporting 2.5 failed, we still need to support it. BUT, I think what we can stop supporting is the odd mixture of development versions (after 2.5, before 3.0), which we have urged people in NG-ARCH to use (to get latest bug fixes).
So, I'll try to remove the crappy code to support 3.0 dev-versions, which we needed in the past. I believe that should significantly simplify the code in question, since now we can rely on --version giving us the right version number. And we have fixed our release process to properly (and at some stage maybe even automatically) support development version numbers (the idea being that e.g. 3.0.1-dev, which we have atm, will be converted to 3.0.0.1 internally in Fab, so we know that this version is between 3.0.0 and 3.1, the next release
There was a problem hiding this comment.
OK, I have cleaned this up a lot ;) I have removed support for PSyclone 2.3.1 and earlier (which did not support --version without a filename specified), and support for unreleased versions after 2.5 but before 3.0. The current handling of version numbers in psyclone is that after a release we will update the version number to the next anticipated one, with -dev added - eg. 3.1.1-dev.
I am just ignore the dev part, since there was a bug in the PSyclone 3.1.0 release, which still reports itself as 3.1.0-dev :( But the intention is that if we do an API change in PSyclone, we will update the dev version number, i.e. it would become say 3.1.1-dev from 3.1.0-dev), so this actually works fine. In general, I will ensure in the psyclone development version to avoid this mess of having an old version number with new behaviour.
I think now that code is pretty clear. There are two somewhat convoluted if-statements, but I think they should stay:
- detect 2.5.0 and newer (which changes the API). This allows Fab to transparently switch between versions, without changing any scripts, e.g. NEMO (which is using 2.5.0) could more easily switch to 3.0 with Fab - a good selling point for Fab :)
- The
processfunction still takes a lot of parameters, which kind of depend on API: using PSyclone as transformation tool (no--psykal-dsl) and DSL. From a user's point of view, it makes a lot of sense to usetransformed_fileas parameter instead ofpsy_file, and vice versa.
| self._api = api | ||
| self._version = None | ||
|
|
||
| def check_available(self) -> bool: |
There was a problem hiding this comment.
Can this whole complication, or a good section of it, be replaced by making use of shutil.which(...)?
There was a problem hiding this comment.
Interesting, I didn't know that tool (I know the shell tool, but afaik that's not available everywhere).
I still need to actually run psyclone, since e.g. on NCI we get:
$ psyclone
/apps/python3/3.12.1/bin/python3.12: error while loading shared libraries: libpython3.12.so.1.0: cannot open shared object file: No such file or directory
$ which psyclone
~/.local/bin/psyclone
$ python3
>>> import shutil
>>> shutil.which("psyclone")
'/home/903/jxh903/.local/bin/psyclone'
So, while indeed this is a kind of broken installation, I can't rely on shutil.which alone. But I'll check if this simplifies the code further.
There was a problem hiding this comment.
Plus, I also still need the version number to switch the command line options between 2.5 and 3.0. But I also think that code is now a lot simpler :)
There was a problem hiding this comment.
I've raised a ticket #393 to investigate this issue further. In the mean time this approach is reasonably neat.
|
I'll try to fix this up and will tag you when I'm done (but for now #390 is really urgent since it breaks all our CI) |
|
Sorry for the delay, besides being busy with the PSyclone 3.1.0 release, I also wanted to make sure Fab works with 3.1.0 :) |
| self._api = api | ||
| self._version = None | ||
|
|
||
| def check_available(self) -> bool: |
There was a problem hiding this comment.
I've raised a ticket #393 to investigate this issue further. In the mean time this approach is reasonably neat.
* Improve support for MPI wrappers (#352) * Update clang binding module. (#376) * Cleaner handling of linking external libraries (#374) * Support additional compilers (#385) * Cray * Icx (LLVM Intel) * nVidia * Psyclone 3 support (#388) * Fixed typo in comment. --------- Co-authored-by: Matthew Hambley <[email protected]>
Supports the new 3.0 release, but also older PSyclone versions.