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.

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

intel-openmp should conflict orther impls #59

Open
ax3l opened this issue Oct 29, 2023 · 14 comments
Open

intel-openmp should conflict orther impls #59

ax3l opened this issue Oct 29, 2023 · 14 comments

Comments

@ax3l
Copy link
Member

ax3l commented Oct 29, 2023

I noticed something similar to the MPI issue with OpenMP since recently: Is it possible that, similar to the recent intel MPI issue, there is another issue that some Python packages started to pull in intel-openmp (or mkl, which requires intel-openmp) and do not clearly mark it as alternative to LLVM OpenMP and GNU OpenMP?

I think that intel-openmp needs to know about libgomp, llvm-openmp and the libomp140.x86_64.dll that is shipped in Visual Studio/VC4 (?) of Conda-Forge? and conflict properly.
https://conda-forge.org/docs/maintainer/knowledge_base.html#openmp

@ax3l
Copy link
Member Author

ax3l commented Oct 29, 2023

I see this currently in Windows builds, where I build with clang-cl for modern OpenMP support using llvm-openmp. This results at runtime in issue of the form

OMP: Error #15: Initializing libomp.dll, but found libomp.dll already initialized.
OMP: Hint This means that multiple copies of the OpenMP runtime have been linked into the program. That is dangerous, since it can degrade performance or cause incorrect results. The best thing to do is to ensure that only a single OpenMP runtime is linked into the process, e.g. by avoiding static linking of the OpenMP runtime in any library. As an unsafe, unsupported, undocumented workaround you can set the environment variable KMP_DUPLICATE_LIB_OK=TRUE to allow the program to continue to execute, but that may cause crashes or silently produce incorrect results. For more information, please see http://openmp.llvm.org/
Fatal Python error: Aborted

since recently. I think the reason is that some Python package I depend on pulls in intel-openmp although it is incompatible with llvm-openmp.

Example: conda-forge/impactx-feedstock#22

@ax3l
Copy link
Member Author

ax3l commented Oct 31, 2023

Are some packages in Conda-Forge unconditionally using mkl these days, which depends on Windows on intel-openmp?

{% if win or 'conda-forge' not in my_channel_targets %}
host:
- intel-openmp {{ openmp_version.split('.')[0] }}.*
- tbb {{ tbb_version.split('.')[0] }}.*
run:
- intel-openmp {{ openmp_version.split('.')[0] }}.*
- tbb {{ tbb_version.split('.')[0] }}.*
{% else %}
host:
- llvm-openmp
- tbb {{ tbb_version.split('.')[0] }}.*
run:
- llvm-openmp
- tbb {{ tbb_version.split('.')[0] }}.*
{% endif %}

Likely scipy:

MKL seems to be the default BLAS/LAPACK provider on Windows now, which makes as its direct dependency intel-openmp the default OpenMP provider on Windows ... OpenMP not documented as such:
https://conda-forge.org/docs/maintainer/knowledge_base.html#openmp

@ax3l
Copy link
Member Author

ax3l commented Oct 31, 2023

attn @leofang @oleksandr-pavlyk @isuruf @jeffhammond for help.

I think two things need to be done (but I do not know how):

  1. intel-openmp should conflict with llvm-openmp and libgomp
  2. the inconsistency in the KB should be fixed:
    • Windows OpenMP defaults are undocumented, but unofficially recommended to use llvm-openmp (via clang-cl)
    • Windows BLAS defaults are MKL which requires intel-openmp

ax3l added a commit to ax3l/amrex-feedstock that referenced this issue Oct 31, 2023
ax3l added a commit to ax3l/pyamrex-feedstock that referenced this issue Oct 31, 2023
@leofang
Copy link
Member

leofang commented Oct 31, 2023

Like mpi being a mutex for the MPI implementation, there's an _openmp_mutex for the OpenMP implementation, see how it's used by libgomp: https://github.com/conda-forge/ctng-compilers-feedstock/blob/086beed486c046f9b205bd94ff35eea74d68ee6a/recipe/meta.yaml#L435-L445.

@leofang
Copy link
Member

leofang commented Oct 31, 2023

Same for llvm-openmp, the mutex needs to be depended on: https://github.com/conda-forge/openmp-feedstock/blob/13d71e73090821d0323deeb57e99d9a13e6cee47/recipe/meta.yaml#L25-L34

cc: @conda-forge/core for vis

@beckermr
Copy link
Member

We need to be careful with the mutexes here. They are not like the mpi ones since we allow swapping openmp at runtime due to ABI compat.

@napetrov
Copy link
Contributor

Are there other packages there we allow swap due to ABI compat? Is there process that we can agree following?

@beckermr
Copy link
Member

BLAS is done that way. We need input from @isuruf on this since he did most of the design here.

@ax3l
Copy link
Member Author

ax3l commented Nov 1, 2023

Additionally, there seems to be a libomp140.x86_64.dll that is a version of OpenMP shipped in Visual Studio itself (?). I cannot use that be cause the supported OpenMP standard is too old.

As a temporary work-around, I tried to rewrite all my feedstocks to use intel-openmp on Windows. This seems to mostly work fine until I reach:
conda-forge/impactx-feedstock#22
Which in some Python pytest tests complains about:

OMP: Error #15: Initializing libiomp5md.dll, but found libomp140.x86_64.dll already initialized.
OMP: Hint This means that multiple copies of the OpenMP runtime have been linked into the program. That is dangerous, since it can degrade performance or cause incorrect results. The best thing to do is to ensure that only a single OpenMP runtime is linked into the process, e.g. by avoiding static linking of the OpenMP runtime in any library. As an unsafe, unsupported, undocumented workaround you can set the environment variable KMP_DUPLICATE_LIB_OK=TRUE to allow the program to continue to execute, but that may cause crashes or silently produce incorrect results. For more information, please see [http://www.intel.com/software/products/support/.](http://www.intel.com/software/products/support/)
Fatal Python error: Aborted

For building, I find OpenMP with CMake's FindOpenMP.cmake.

@leofang
Copy link
Member

leofang commented Nov 1, 2023

We need to be careful with the mutexes here. They are not like the mpi ones since we allow swapping openmp at runtime due to ABI compat.

@beckermr I am not sure I follow. Regardless of MPI/OpenMP/BLAS, the purpose of the mutexes is the same: Disallowing multiple implementations from being installed to the same conda environment. How the mutex is implemented is an implementation detail that only package maintainers need to know.

In the case of MPI, we need to add a string identifier to mpi for a new MPI implementation.

In the case of BLAS, we need to register the BLAS implementation in the blas-feedstock.

In the case of OpenMP, for some reason unclear to me the mutex publishing happens in two feedstocks, ctng-compiler (for gomp) and _openmp_mutex (for llvm-openmp).

But in the end, all such packages work with the same concept. Am I missing something?

It's worth noting that @h-vetinari also reported this very same issue 2 years ago: conda-forge/_openmp_mutex-feedstock#7 We definitely should fix it.

@isuruf
Copy link
Member

isuruf commented Nov 2, 2023

Regarding ABI, we have intel-openmp because llvm-openmp does not have the VCOMP symbols in them which leads to pytorch on windows breaking. VCOMP is old and going away in favour of LLVM openmp (even MSVC supports LLVM openmp now). We really should have a pytorch win build in conda-forge and then we can remove intel-openmp.
Intel and LLVM OpenMP share code except for the VCOMP compatibility options.

Yes, we can have intel-openmp and llvm-openmp conflict with each other for now.

@milubin
Copy link

milubin commented Nov 2, 2023

@isuruf As mentioned above MKL depends on intel-openmp on Windows. Does intel-openmp need to be replaced with llvm-openmp in MKL too?

@ax3l
Copy link
Member Author

ax3l commented Nov 10, 2023

Hi, is there any progress on this, e.g., marking the conflict for `intel-openmp? :)

@Krande
Copy link

Krande commented Jun 6, 2024

Hi, we are also seeing issues for us when developing Windows support for Code Aster due to the clobbering of files when mixing intel-openmp and llvm-openmp when trying to use intel-fortran-rt (depends on llvm-openmp) together with mkl (depends on intel-openmp). This is ultimately causing dll issues for us. I can fix it manually by doing mamba remove intel-openmp llvm-openmp --force and then do mamba install intel-openmp. But that's not really solving our issue :(

Would it be possible to switch from intel to llvm openmp for mkl or llvm to intel for intel-fortran-rt? Alternatively, create variants of mkl and intel-fortran-rt that supports both?

I can try to help with a PR with a little guidance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants