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).
| # 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
| 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.
| if api and api.lower() == "nemo": | ||
| api = "" |
There was a problem hiding this comment.
Rather than trying to make on class support quite different versions of the tool would it make sense to have an old PSyclone and a new PSyclone class with some sort of factory process to choose between them?
There was a problem hiding this comment.
I would hope that this sequence of tests is not too bad. Yes, if we had two version, the if-statement would only have half the size (one version would contain the 'if' tests, which still need to test for new-style parameters, the other version the else branch with the old-style parameter), we would get quite a bit of either duplicated code, or if we create a base class, more files. I would think that this actually adds more overhead compared with one if-statement, but I am happy to re-evaluate once I have removed support for 3.0-dev versions.
|
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) |
Supports the new 3.0 release, but also older PSyclone versions.