KratosMultiphysics / Kratos

Kratos Multiphysics (A.K.A Kratos) is a framework for building parallel multi-disciplinary simulation software. Modularity, extensibility and HPC are the main objectives. Kratos has BSD license and is written in C++ with extensive Python interface.
https://kratosmultiphysics.github.io/Kratos/
Other
1.03k stars 245 forks source link

[Python] Supporting external libraries in python >=3.8 [Intel-mkl] #10240

Closed sunethwarna closed 1 year ago

sunethwarna commented 2 years ago

Description Python 3.8 and later versions has removed loading libraries by going through %PATH% variable in windows, therefore the usual calls such as intel-mkl vars.bat cannot be used anymore. python3.8 and later versions has introduced os.add_dll_directory where the dll lib paths needs to be added for runtime linking.

https://docs.python.org/3/library/os.html#os.add_dll_directory

For the time being, we can put a warning in CmakeLists.txt file so when someone encounters this problem, they can be aware. I am open for clear workarounds.

Scope

roigcarlo commented 2 years ago

Sadly my intel system is currently broken and I have no means to test this fast.

Can you try to add the paths in %PATH% through add_dll_directory() in LinearSolvers init or the Kratos init?

sunethwarna commented 2 years ago

@roigcarlo It works, but its a solution from the user end. Thats why we may need to put an warning so users are aware.

roigcarlo commented 2 years ago

Users should not touch this. We should add a system to automatically fetch the variables either directly from path or to a given variable defined by us.

For what I can read this is now the standard system, so we have to internally provide support for that.

I am thinking something in the lines of:

import os
import sys
if sys.version_info >= (3, 8):
    os.add_dll_directory(os.environ['PATH']) # or os.add_dll_directory(os.environ['KRATOS_PATH'])
sunethwarna commented 2 years ago

One woking solution is using an environment handler like conda environements and using intel-mkl-devel inside that. It links correctly within the environment even with python3.9. But if someone wants to download intel mkl and run it using the vars.bat, it won't work anymore.

We can add [os.add_dll_directory(dll_dir) for dll_dir in os.environ['PATH'].split(";")] to the kernel loading point where we set paths for python extension libs. But using %PATH% is what windows python wants to avoid. If someone only usesKratosMultiphysics` lib, then it will work. This is only a problem when there are dynamic runtime linking in Kratos libs.

I see two opiotns:

  1. Remove support for standalone intel-mkl liek libs. [I am not sure what other libs are used in windows, maybe mmg?] and advocate to use python environments like anaconda.
  2. Add [os.add_dll_directory(dll_dir) for dll_dir in os.environ['PATH'].split(";")] to the kernel loading point.

What are the dynamically linked libs used in windows usually?

roigcarlo commented 2 years ago

As I said, I would go for the sencond. We already manually add the lib cores for the apps, and pip should handle correctly when installing packages.

I have tried the MKL packages in pip and conda and both seem to work. Also intel is not yet oficially supported, so is a developer choice. My reasoning for stick with the PATH is basically that the behabiour remains the same.

We can think latter on a more pythonic solution, but as for now I would keep the current behaviour.

P.s. we can make it portable with os.pathsep for the splitting.

roigcarlo commented 1 year ago

@AlejandroCornejo This affects you, and pretty much everyone since we upgraded min to 3.8.

@KratosMultiphysics/technical-committee We should dice on this. I double back into adding the PATH paths to the load paths in the Kratos init function

AlejandroCornejo commented 1 year ago

Hi all, Today I have been trying to upgrade my python version from 3.7 to 3.10. It is important to me to use the Intel MKL library to use Pardiso.

The reality is that from python 3.8 on it is not working (DLL load fails when activating the MKL in the compilation) together with MKL.

If I set -DUSE_EIGEN_MKL=OFF it works properly even with python 3.10 (expected). So there is something bad between MKL and python above 3.7...

AlejandroCornejo commented 1 year ago

In this regard, I'd ask to enforce the 3.8 version of python once we kind of solve this... Otherwise a lot of people will be affected

rubenzorrilla commented 1 year ago

In this regard, I'd ask to enforce the 3.8 version of python once we kind of solve this... Otherwise a lot of people will be affected

See #10428.

AlejandroCornejo commented 1 year ago

Just to update, if we put os.add_dll_directory("_path_to_mkl_dll") it loads the solver but it crashes when solving (with or without pardiso solver) (no message shown in FullDebug sorry)

I have checked that the case is running properly in another machin with python 3.7 and mkl (I'm using windows always)