Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

avoid warnings being treated as errors when installing imkl-FFTW with Intel OneAPI compilers and RPATH linking #2912

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

avoid warnings being treated as errors when installing imkl-FFTW with Intel OneAPI compilers and RPATH linking #2912

wants to merge 1 commit into from

Conversation

bedroge
Copy link
Contributor

@bedroge bedroge commented Mar 21, 2023 •
edited

Fixes #2910.

@bedroge bedroge mentioned this pull request Mar 21, 2023
Open
@bedroge
Copy link
Contributor Author

bedroge commented Mar 21, 2023

Test report by @bedroge

Overview of tested easyconfigs (in order)

  • SUCCESS imkl-FFTW-2022.2.1-iimpi-2022b.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
bob-Latitude-5300 - Linux Ubuntu 22.04, x86_64, Intel(R) Core(TM) i7-8665U CPU @ 1.90GHz, Python 3.10.6
See https://gist.github.com/bedroge/4433cb44def303f477a1b3a8dcd1ed5b for a full test report.

@boegel boegel added this to the next release (4.7.2?) milestone Mar 28, 2023
@boegel boegel changed the title Fix for imkl-FFTW when building with Intel OneAPI compilers and RPATH linking avoid warnings being treated as errors when installing imkl-FFTW with Intel OneAPI compilers and RPATH linking Apr 12, 2023
boegel
boegel approved these changes Apr 12, 2023
View reviewed changes
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@boegel
Copy link
Member

boegel commented Apr 12, 2023

Test report by @boegel

Overview of tested easyconfigs (in order)

  • SUCCESS imkl-FFTW-2021.4.0-gompi-2021b.eb
  • SUCCESS imkl-FFTW-2021.4.0-iimpi-2021b.eb
  • SUCCESS imkl-FFTW-2022.2.1-iimpi-2022b.eb

Build succeeded for 3 out of 3 (3 easyconfigs in total)
node3101.skitty.os - Linux RHEL 8.6, x86_64, Intel(R) Xeon(R) Gold 6140 CPU @ 2.30GHz (skylake_avx512), Python 3.6.8
See https://gist.github.com/boegel/cf9bbf2fb8a620bb0cf0f3cafa6551e1 for a full test report.

@boegel
Copy link
Member

boegel commented Apr 12, 2023 •
edited

Test report by @boegel

Overview of tested easyconfigs (in order)

Build succeeded for 0 out of 1 (1 easyconfigs in total)
node3101.skitty.os - Linux RHEL 8.6, x86_64, Intel(R) Xeon(R) Gold 6140 CPU @ 2.30GHz (skylake_avx512), Python 3.6.8
See https://gist.github.com/boegel/42a8e2f97b73d39a673c5b90c7cc098e for a full test report.

edit: tested with eb --rpath --read-only-installdir

@boegel
Copy link
Member

boegel commented Apr 12, 2023 •
edited

@bedroge My last test report with --rpath failed because I also have EasyBuild configured with --read-only-installdir, so before you try to patch makefile you need to make sure that file is writable, and the directory it is in, is writable:

== 2023-04-12 18:50:48,339 filetools.py:1665 INFO Applying following regex substitutions to ['/software/imkl/2022.2.1/mkl/2022.2.1/interfaces/fftw2x_cdft/makefile']: [('-Werror', '-Werror -Wno-unused-command-line-argument')]
== 2023-04-12 18:50:48,340 filetools.py:1933 DEBUG Not creating existing path /software/imkl/2022.2.1/mkl/2022.2.1/interfaces/fftw2x_cdft
== 2023-04-12 18:50:48,555 build_log.py:171 ERROR EasyBuild crashed with an error (at easybuild/easybuild-framework/easybuild/base/exceptions.py:126 in __init__): Failed to copy file /software/imkl/2022.2.1/mkl/2022.2.1/interfaces/fftw2x_cdft/makefile to /software/imkl/2022.2.1/mkl/2022.2.1/interfaces/fftw2x_cdft/makefile.orig.eb: [Errno 13] Permission denied: '/software/imkl/2022.2.1/mkl/2022.2.1/interfaces/fftw2x_cdft/makefile.orig.eb' (at easybuild/easybuild-framework/easybuild/tools/filetools.py:2439 in copy_file)

edit: hmm, not sure this actually explains what's going on, because the changing of permissions triggered by using --read-only-installdir is only done in the permissions step which is run after the post_install step...

@easybuilders easybuilders deleted a comment from nurul8888 Apr 20, 2023
@boegel boegel modified the milestones: 4.7.3, release after 4.7.3 Jul 5, 2023
@bedroge bedroge mentioned this pull request Aug 4, 2023
Merged
@casparvl
Copy link
Contributor

casparvl commented Aug 7, 2023 •
edited

Ugh, I did:

eb imkl-FFTW-2023.1.0-iimpi-2023a.eb imkl-FFTW-2023.1.0-iimpi-2023.03.eb imkl-FFTW-2023.0.0-iimpi-2022.12.eb imkl-FFTW-2022.2.1-iimpi-2022b.eb imkl-FFTW-2022.2.1-iimpi-2022.11.eb imkl-FFTW-2022.2.0-iimpi-2022.09.eb imkl-FFTW-2022.1.0-iimpi-2022a.eb imkl-FFTW-2021.4.0-iimpi-2021b.eb --accept-eula-for=Intel-oneAPI --include-easyblocks-from-pr 2912 -r --rebuild --upload-test-report

Builds were succesful, but failed at uploading test report: urllib.error.HTTPError: HTTP Error 502: Bad Gateway. Anyway, hereby: the same list on the same system as #2975 (comment) completed succesfully with this PR.

I'll retrigger the build, it should take about 90 minutes or so. See if it uploads succesfully then...

@casparvl
Copy link
Contributor

casparvl commented Aug 7, 2023

Test report by @casparvl

Overview of tested easyconfigs (in order)

  • SUCCESS imkl-FFTW-2023.1.0-iimpi-2023a.eb
  • SUCCESS imkl-FFTW-2023.1.0-iimpi-2023.03.eb
  • SUCCESS imkl-FFTW-2023.0.0-iimpi-2022.12.eb
  • SUCCESS imkl-FFTW-2022.2.1-iimpi-2022b.eb
  • SUCCESS imkl-FFTW-2022.2.1-iimpi-2022.11.eb
  • SUCCESS imkl-FFTW-2022.2.0-iimpi-2022.09.eb
  • SUCCESS imkl-FFTW-2022.1.0-iimpi-2022a.eb
  • SUCCESS imkl-FFTW-2021.4.0-iimpi-2021b.eb

Build succeeded for 8 out of 8 (8 easyconfigs in total)
tcn375.local.snellius.surf.nl - Linux RHEL 8.6, x86_64, AMD EPYC 7H12 64-Core Processor, Python 3.6.8
See https://gist.github.com/casparvl/7f7fc6eb2c3180d1ae4ab4b8a7480a94 for a full test report.

boegel
boegel requested changes Aug 8, 2023
View reviewed changes
easybuild/easyblocks/i/imkl.py
# Here, we patch the makefiles and add -Wno-unused-command-line-argument to avoid these warnings alltogether.
if get_software_root('intel-compilers') and build_option('rpath'):
if self.toolchain.options.get('oneapi') or self.toolchain.options.get('oneapi_c_cxx'):
regex_icx_subs = [('-Werror', '-Werror -Wno-unused-command-line-argument')]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it makes more sense to replace -Wall than -Werror, since the order of options may matter.

When replacing -Werror, I think we assume that -Wall is listed earlier, so -Wno-unused-* can disable the warning.
When replacing -Wall to inject -Wno-unused-*, we're disabling that specific warning right after enabling all warnings (and before -Werror which enables all warnings to be treated as errors kicks in).

This may make this fix a bit more robust w.r.t. changes to the makefile being patched

regex_icx_subs = [('-Wall', '-Wall -Wno-unused-command-line-argument')]

@boegel
Copy link
Member

boegel commented Aug 8, 2023

@bedroge My last test report with --rpath failed because I also have EasyBuild configured with --read-only-installdir, so before you try to patch makefile you need to make sure that file is writable, and the directory it is in, is writable:

== 2023-04-12 18:50:48,339 filetools.py:1665 INFO Applying following regex substitutions to ['/software/imkl/2022.2.1/mkl/2022.2.1/interfaces/fftw2x_cdft/makefile']: [('-Werror', '-Werror -Wno-unused-command-line-argument')]
== 2023-04-12 18:50:48,340 filetools.py:1933 DEBUG Not creating existing path /software/imkl/2022.2.1/mkl/2022.2.1/interfaces/fftw2x_cdft
== 2023-04-12 18:50:48,555 build_log.py:171 ERROR EasyBuild crashed with an error (at easybuild/easybuild-framework/easybuild/base/exceptions.py:126 in __init__): Failed to copy file /software/imkl/2022.2.1/mkl/2022.2.1/interfaces/fftw2x_cdft/makefile to /software/imkl/2022.2.1/mkl/2022.2.1/interfaces/fftw2x_cdft/makefile.orig.eb: [Errno 13] Permission denied: '/software/imkl/2022.2.1/mkl/2022.2.1/interfaces/fftw2x_cdft/makefile.orig.eb' (at easybuild/easybuild-framework/easybuild/tools/filetools.py:2439 in copy_file)

edit: hmm, not sure this actually explains what's going on, because the changing of permissions triggered by using --read-only-installdir is only done in the permissions step which is run after the post_install step...

After looking at this again with @casparvl, I now understand this better.

The patching of the makefile is being done in the imkl installation directory when installing imkl-FFTW.
That explains the Permission denied when EasyBuild is configuring with --read-only-installdir: it's correctly pointing out that imkl-FFTW has no business changing stuff in the imkl installation.

The changes in this PR don't cause that, but they do point out a problem with the imkl-FFTW easyblock: it shouldn't be running the build of the FFTW interfaces from the imkl installation directory, but it should copy the necessary files to the imkl-FFTW build directory first, and run the build from there instead.

With that in mind, I think we shouldn't merge this PR as is yet, we should tackle that problem first (to avoid making a bad situation worse).
We can merge the alternative fix in #2975 as a short-term solution for the problem being fixed, but I think in general the approach taken here (patching the makefile to -Wno-unused-* is added) is preferably over just blindly adding -Wno-unused-* to $CFLAGS as is done in #2975.

@boegel boegel modified the milestones: 4.9.1, release after 4.9.1 Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@boegel boegel boegel requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
bug fix
Projects
None yet
Milestone
release after 4.9.1
Development

Successfully merging this pull request may close these issues.

RPATH issue with cdft wrappers when using OneAPI compilers
3 participants
@bedroge @boegel @casparvl