Add exceptions for FFTW/3.3.6 on POWER with GCC 5/6/7 #1274
Conversation
easybuild/easyblocks/f/fftw.py
Outdated
| fftw_version = self.version | ||
| if cpu_arch in [POWER] and self.toolchain.comp_family() in [TC_CONSTANT_GCC] and \ | ||
| LooseVersion(fftw_version) == LooseVersion("3.3.6"): | ||
| # See https://github.com/FFTW/fftw3/issues/59 which applies to GCC 5/6/7 |
There was a problem hiding this comment.
@boegel There is no easy way to check the GCC version to be used in this easyblock, at the time this list of options is created the dependencies are not loaded.
There was a problem hiding this comment.
please use == rather than in if there's only one thing to compare with, and try to avoid line wrapping
Also, shouldn't you use <= rather than == for the FFTW version? In addition, are you sure the problem is going to be fixed in the next FFTW release?
cpu_arch = get_cpu_architecture()
comp_fam = self.toolchain.comp_family()
fftw_ver = LooseVersion(self.version)
if cpu_arch == POWER and comp_fam == TC_CONSTANT_GCC and fftw_ver <= LooseVersion('3.3.6'):There was a problem hiding this comment.
W.r.t. to the GCC version, that would be tricky to check at this stage, indeed.
Not sure if we should worry about it too much though, GCC 4.x is quite old now, and unlikely to be used on POWER anyway?
What kind of problems would you run into when using GCC 4.x?
You could add a check for the GCC version being used in the prepare method instead, and error out if it's too old for POWER, at that point the toolchain is loaded.
There was a problem hiding this comment.
So the system GCC on the Power system I'm using is 4.8.5 and my understanding is that these flags work fine with GCC4.8 and GCC4.9 (see FFTW/fftw3#59 (comment) )...but I haven't checked myself and I'm not going to.
Leaving a blanket GCC check like the above is fine since they will do harm for GCC4 (apart from maybe make it a little slower) and we are not expecting people to willingly chose an old compiler on Power.
There was a problem hiding this comment.
I don't have a guarantee the problem will be fixed in the next release but the bug is reported ( FFTW/fftw3#59 ) and I would prefer not to turn off these optimisations by default but rather run into the problem and then see that I need to include 3.3.7 or whatever. For earlier versions than 3.3.6, I don't know if there is a problem or not so I err on the side of caution (but I am happy to change it).
There was a problem hiding this comment.
OK on both not checking GCC version & only doing this for FFTW 3.3.6 for now.
easybuild/easyblocks/f/fftw.py
Outdated
| fftw_version = self.version | ||
| if cpu_arch in [POWER] and self.toolchain.comp_family() in [TC_CONSTANT_GCC] and \ | ||
| LooseVersion(fftw_version) == LooseVersion("3.3.6"): | ||
| # See https://github.com/FFTW/fftw3/issues/59 which applies to GCC 5/6/7 |
There was a problem hiding this comment.
please use == rather than in if there's only one thing to compare with, and try to avoid line wrapping
Also, shouldn't you use <= rather than == for the FFTW version? In addition, are you sure the problem is going to be fixed in the next FFTW release?
cpu_arch = get_cpu_architecture()
comp_fam = self.toolchain.comp_family()
fftw_ver = LooseVersion(self.version)
if cpu_arch == POWER and comp_fam == TC_CONSTANT_GCC and fftw_ver <= LooseVersion('3.3.6'):
easybuild/easyblocks/f/fftw.py
Outdated
| fftw_version = self.version | ||
| if cpu_arch in [POWER] and self.toolchain.comp_family() in [TC_CONSTANT_GCC] and \ | ||
| LooseVersion(fftw_version) == LooseVersion("3.3.6"): | ||
| # See https://github.com/FFTW/fftw3/issues/59 which applies to GCC 5/6/7 |
There was a problem hiding this comment.
W.r.t. to the GCC version, that would be tricky to check at this stage, indeed.
Not sure if we should worry about it too much though, GCC 4.x is quite old now, and unlikely to be used on POWER anyway?
What kind of problems would you run into when using GCC 4.x?
You could add a check for the GCC version being used in the prepare method instead, and error out if it's too old for POWER, at that point the toolchain is loaded.
easybuild/easyblocks/f/fftw.py
Outdated
| if cpu_arch in [POWER] and self.toolchain.comp_family() in [TC_CONSTANT_GCC] and \ | ||
| LooseVersion(fftw_version) == LooseVersion("3.3.6"): | ||
| # See https://github.com/FFTW/fftw3/issues/59 which applies to GCC 5/6/7 | ||
| if (prec == 'single'): |
There was a problem hiding this comment.
drop the (...), not needed:
if prec == 'single':| # append additional configure options (may be empty string, but that's OK) | ||
| self.cfg.update('configopts', [' '.join(prec_configopts) + common_config_opts]) | ||
| self.cfg.update('configopts', [' '.join(prec_configopts) + ' ' + common_config_opts]) |
There was a problem hiding this comment.
Since this is a bug fix, maybe this should be done in a separate PR? Trivial to merge...
|
Tested with existing FFTW easyconfig, and confirmation that it works as intended via test report in easybuilders/easybuild-easyconfigs#5242 |
No description provided.