search

coin-or / Ipopt

COIN-OR Interior Point Optimizer IPOPT
https://coin-or.github.io/Ipopt
Other
1.41k stars 281 forks source link

Unable to load hsl solvers with ipopt 3.14.15 #759

Closed S-Dafarra closed 5 months ago

S-Dafarra commented 5 months ago

I have installed ipopt 3.14.15 with conda on Windows.

If I try to use a hsl linear solver, I get the following exception:

Exception of type: DYNAMIC_LIBRARY_FAILURE in file "D:\bld\ipopt_1712819048605\work\src\Common\IpLibraryLoader.cpp" at line 60:
 Exception message: Error 87 while loading DLL libhsl.dll: The parameter is incorrect.

If I use the 3.14.14 version instead it works fine.

svigerske commented 5 months ago

Damn it. The dynamic loading of libraries like HSL on Windows is the one thing that changed from 3.14.14 to 3.14.15 (#755).

Can you say more about your setup? Where do you have the HSL DLL located, from where do you run Ipopt, and do you have the path to the HSL DLL in the PATH environment variable?

S-Dafarra commented 5 months ago

do you have the path to the HSL DLL in the PATH environment variable?

Yes!

In fact it was puzzling me. The error when it does not find the library is 

Exception of type: DYNAMIC_LIBRARY_FAILURE in file "D:\bld\ipopt_1705695044464\work\src\Common\IpLibraryLoader.cpp" at line 59:
 Exception message: Error 126 while loading DLL libhsl.dll: The specified module could not be found.

The same error is also thrown when Windows does not find some runtime dependency of libhsl.dll, while in this case the error code is different.

Where do you have the HSL DLL located, from where do you run Ipopt

ipopt is installed via conda, while libhsl.dll is in a personal path. Both of them are in the PATH.

The code I was trying to run is a bit complex, involving other dependencies (like casadi). If necessary I can try to work on a minimal reproducible example.

svigerske commented 5 months ago

Hmm, one would think that this setup would work. I have no clue what it means by a parameter being incorrect.

An original suggestion in #755 was to try both LoadLibrary (as in 3.14.14) and LoadLibraryExA (as in 3.14.15). I've done this in 9a36a9ed and here are windows DLLs: https://cloud.gams.com/s/PCZY3RnGKZj5bND If you like, you could try whether these work. I don't know how the conda libraries are build.

traversaro commented 5 months ago

Thanks @svigerske for the quick reply ! I backported the patch in conda binaries for 3.14.15 in https://github.com/conda-forge/ipopt-feedstock/pull/103, I also enable upload of test artifacts (see https://conda-forge.org/docs/maintainer/updating_pkgs/#downloading-prebuilt-packages-from-ci), so @S-Dafarra you can test if that PR solve your problem (just install with conda install <downloaded_package>.conda the downloaded package).

I don't know how the conda libraries are build.

Unfortunately at the moment we are using a custom CMake script for ipopt mantained at https://github.com/conda-forge/ipopt-feedstock/blob/main/recipe/cmake/CMakeLists.txt . It would be great to be aligned with upstream, but to be honest I never looked into how to make autotools works with the rest of the Windows conda-forge machinery.

For what regards HSL's dll, actually those are built with a fix we did to HSL's upstream meson build system, we actually still need to report that fix upstream.

traversaro commented 5 months ago

Actually I had checked https://github.com/coin-or/Ipopt/pull/755 back when it was proposed, and it seemed fine. However, looking into the details of the documentation I noticed that in https://learn.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibraryexa it reads that if LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR option is used, "The lpFileName parameter must specify a fully qualified path.". Perhaps this is the problem? @S-Dafarra can you try to pass the absolute path to the libhsl.dll ? If that is the case, I guess it make sense to use https://github.com/coin-or/Ipopt/commit/9a36a9ed0d19900cbc7553209de4d0207c731afa . Or perhaps we should check if libname is an absolute path, and just use LoadLibraryExA in that case?

svigerske commented 5 months ago

Oh, I read through this documentation several times, and still overlooked that small sentence.

So if libname is an absolute path ("fully qualified path"), one should use LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR, otherwise use LOAD_LIBRARY_SEARCH_DEFAULT_DIRS. But one cannot combine both (at least not with the default relative path for libname).

It should be sufficient to check libname[1] == ':' to recognize an absolute path (?).

svigerske commented 5 months ago

In the next release, LoadLibraryExA with LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR will only be used if libname seems to be an absolute path. Otherwise, it will be as before 3.14.15, i.e., LoadLibraryA is used.

This seems to work on my system when specifying a library as absolute path or when having it in PATH.

CC @metab0t

traversaro commented 5 months ago

Thanks for the fix!

It should be sufficient to check libname[1] == ':' to recognize an absolute path (?).

Sorry, I missed that comment. I am not sure how widespread those are, but I guess that check may be fail when using UNC or DOS device path (see https://learn.microsoft.com/en-us/dotnet/standard/io/file-path-formats#unc-paths). Anyhow, I guess those are kind of seldomly used, and anyhow if they are used the normal LoadLibraryA that was used before 3.14.15 will be used, so I guess this is ok.

metab0t commented 5 months ago

@svigerske Sorry for breaking existing code because of the mess of DLL search strategy on Windows. Instead of judging whether the path is absolute, can we just use the initial commit? Search the default paths, if not found, then use LoadLibraryExA as a possible fix? Besides, we should use LOAD_LIBRARY_SEARCH_DEFAULT_DIRS | LOAD_LIBRARY_SEARCH_DLL_LOAD_DIRS in LoadLibraryExA instead of LOAD_LIBRARY_SEARCH_DEFAULT_DIRS because the latter does not search default paths at all. libhsl.dll may have some dependencies in default search paths. I also make a fix in my own package: https://github.com/metab0t/PyOptInterface/commit/94c6561bbdcf4cd3480166c3af29a2c66b7042d9

svigerske commented 5 months ago

I was changing it to LOAD_LIBRARY_SEARCH_DLL_LOAD_DIRS for the call to LoadLibraryExA. The LOAD_LIBRARY_SEARCH_DEFAULT_DIRS adds some additional directories. But using LoadLibraryExA with any non-zero argument prevents it from looking into PATH, which we had before 3.14.15. So we still need the LoadLibraryA call.

We can do as in the initial commit, just need to improve error handling. At the moment, the message from the LoadLibrary call would be lost, if that fails.

metab0t commented 5 months ago

Yes, I agree. LoadLibraryA handles the libhsl.dll cases and LoadLibraryExA for the full DLL paths.

S-Dafarra commented 5 months ago

@S-Dafarra can you try to pass the absolute path to the libhsl.dll ?

By passing the absolute path using the hsllib option, it works

S-Dafarra commented 5 months ago

so @S-Dafarra you can test if that PR solve your problem (just install with conda install <downloaded_package>.conda the downloaded package).

I have tested the version mentioned in https://github.com/conda-forge/ipopt-feedstock/pull/103#issuecomment-2067222479 and it also works with the default relative setting. Thanks a lot @svigerske and @traversaro for the super quick support!

amontoison commented 5 months ago

For what regards HSL's dll, actually those are built with a fix we did to HSL's upstream meson build system, we actually still need to report that fix upstream.

@traversaro Don't hesitate to contact me if you have an issue the new Meson build system of COIN-HSL / libHSL.

traversaro commented 5 months ago

For what regards HSL's dll, actually those are built with a fix we did to HSL's upstream meson build system, we actually still need to report that fix upstream.

@traversaro Don't hesitate to contact me if you have an issue the new Meson build system of COIN-HSL / libHSL.

Great! I sent you a mail.