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

Fix libblastrampoline with Ipopt_jll #163

Merged
merged 4 commits into from
Feb 20, 2024
Merged

Fix libblastrampoline with Ipopt_jll #163

merged 4 commits into from
Feb 20, 2024

Conversation

odow
Copy link
Member

@odow odow commented Mar 22, 2023

image

@odow
Copy link
Member Author

odow commented Mar 22, 2023

As a note to my future self, here's a drop in replacement for Ipopt_jll.amplexe that we could add to Ipopt:

import OpenBLAS32_jll
import Ipopt_jll

function amplexe(f)
    lbt_default_libs = get(ENV, "LBT_DEFAULT_LIBS", nothing)
    if lbt_default_libs === nothing
        ENV["LBT_DEFAULT_LIBS"] = OpenBLAS32_jll.libopenblas
    end
    ret = Ipopt_jll.amplexe(f)
    if lbt_default_libs === nothing
        delete!(ENV, "LBT_DEFAULT_LIBS")
    else
        ENV["LBT_DEFAULT_LIBS"] = lbt_default_libs
    end
    return ret
end

src/AmplNLWriter.jl Outdated Show resolved Hide resolved
@odow
Copy link
Member Author

odow commented Mar 22, 2023

Aaaaand. Windows is the odd duck

https://github.com/jump-dev/AmplNLWriter.jl/actions/runs/4486368238/jobs/7888794896?pr=163

@giordano
Copy link

As a note to my future self, here's a drop in replacement for Ipopt_jll.amplexe that we could add to Ipopt:

As a general remark, I'd warn you against manipulating ENV directly: that's an unsafe global state, should something fail while you're editing it you can be left in a nasty state. Instead, use addenv, which also affects only sub-processes.

src/AmplNLWriter.jl Outdated Show resolved Hide resolved
@odow
Copy link
Member Author

odow commented Mar 22, 2023

As a general remark, I'd warn you against manipulating ENV directly

Sure. I don't think we'd advise using it in anything critical. (The code here uses addenv.)

It's more for the occasions when I want to run the binary quickly in the REPL to test something.

@odow
Copy link
Member Author

odow commented Mar 22, 2023

So all we get is an unhelpful could not spawn setenv('C:\\Users\\runneradmin\\.julia\\artifacts\\774ef6a9c59cbea84ec6a86bf6d7a2dc39eff2e6\\bin\\ipopt.exe` error.

The complication is that we use openblas on Windows:
https://github.com/JuliaPackaging/Yggdrasil/blob/d3cf664e736225391b0e0dbffbec28ce20688434/C/Coin-OR/Ipopt/build_tarballs.jl#L28-L32
and LBT on others
https://github.com/JuliaPackaging/Yggdrasil/blob/d3cf664e736225391b0e0dbffbec28ce20688434/C/Coin-OR/Ipopt/build_tarballs.jl#L68-L69

But it seems like the original reason is now fixed, JuliaLinearAlgebra/libblastrampoline#89, JuliaLang/julia#47638, so we could try rebuilding Ipopt.

@odow
Copy link
Member Author

odow commented Mar 22, 2023

x-ref: JuliaPackaging/Yggdrasil#6441

src/AmplNLWriter.jl Outdated Show resolved Hide resolved
src/AmplNLWriter.jl Outdated Show resolved Hide resolved
@odow
Copy link
Member Author

odow commented Mar 23, 2023

@staticfloat any ideas? Our ipopt.exe binary uses OpenBLAS32_jll directly and doesn't use LBT. I just get a 'could not spawn' error: https://github.com/jump-dev/AmplNLWriter.jl/actions/runs/4495954495/jobs/7910133289?pr=163#step:6:173

MOI.get(model, MOI.RawStatusString()) = "Error calling the solver. Failed with: Base.IOError(\"could not spawn `'C:\\\\Users\\\\runneradmin\\\\.julia\\\\artifacts\\\\774ef6a9c59cbea84ec6a86bf6d7a2dc39eff2e6\\\\bin\\\\ipopt.exe' 'C:\\\\Users\\\\RUNNER~1\\\\AppData\\\\Local\\\\Temp\\\\jl_eXBXKp\\\\model.nl' -AMPL 'option_file_name=D:\\\\a\\\\AmplNLWriter.jl\\\\AmplNLWriter.jl\\\\test\\\\ipopt.opt' print_level=1`: unknown error (UNKNOWN)\", -4094)"

I don't have a windows machine, so I'm a but out of luck.

We know that the LibraryProduct works fine on Windows, because that's what https://github.com/jump-dev/Ipopt.jl is. This is just a problem with the ExecutableProduct.

@staticfloat
Copy link
Contributor

I don't really know, sorry. It's weird to me that there are so many backslashes.... is there some kind of path escaping going on?

@amontoison
Copy link
Collaborator

@odow I recompiled MUMPS with LBT for Julia >= 1.9.
I will do the same thing with Ipopt.

I propose to not support anymore the artifacts with partial support of LBT when Julia 1.9 will be released.

@odow
Copy link
Member Author

odow commented Mar 27, 2023

Okay. I'll wait for that release before spending more time trying to debug this

@odow
Copy link
Member Author

odow commented Mar 27, 2023

is there some kind of path escaping going on?

I tried removing some of the escapes. But I think it's just a printing artifact of how we're catching the exception, storing it as a string, and then printing it.

@odow
Copy link
Member Author

odow commented Mar 27, 2023

It can't even spawn $exe -v, so something is massively wrong.

@amontoison
Copy link
Collaborator

amontoison commented Mar 31, 2023

@odow For Windows we don't need to modify something because I precompiled the last Ipopt_jll with OpenBLAS32_jll. If the old version of Ipopt_jll 300.1400.400 is working, I don't understand why something changed with the last artifact.
We only need to provide additional arguments for LBT when !Sys.windows().

@odow odow closed this May 10, 2023
@odow odow reopened this May 10, 2023
@odow
Copy link
Member Author

odow commented May 10, 2023

Let's see how JuliaPackaging/Yggdrasil#6734 goes.

@odow
Copy link
Member Author

odow commented May 10, 2023

@amontoison is used the new version:
image
but what are we missing?

It was compiled with https://buildkite.com/julialang/yggdrasil/builds/2943#01880319-dbcd-47ac-bf17-436ae373a5d4
image
but it used OpenBLAS 0.3.21. Could that be a problem?

It's using MUMPS 500.500.101, which is JuliaPackaging/Yggdrasil#5599, which uses OpenBLAS on Windows.

@amontoison
Copy link
Collaborator

@odow
I found this message in the logs: https://buildkite.com/julialang/yggdrasil/builds/2943#01880319-dbcd-47ac-bf17-436ae373a5d4/638-1459

@odow
Copy link
Member Author

odow commented May 10, 2023

Hoorah. Only. What does that mean.

@amontoison
Copy link
Collaborator

amontoison commented May 10, 2023

@odow
I recompiled the old release 3.14.4 on Windows (with the last release of OpenBLAS32), can you test with my fork https://github.com/amontoison/Ipopt_jll.jl ?
The version of the artifact is 300.1400.1066 (3.14.4 + 666 👿)

@amontoison
Copy link
Collaborator

amontoison commented May 10, 2023

Can you add a println(Base.BinaryPlatforms.host_triplet()) to check the platform of the Windows builds?
It seems that Ipopt 3.14.4 is not working with the updated dependencies.
The culprit is OpenBLAS32 or MUMPS_seq.

@odow
Copy link
Member Author

odow commented May 10, 2023

x86_64-w64-mingw32-libgfortran5-cxx11-julia_version+1.9.0

It pulled in Julia v1.9.0?

end
path() do exe
run(`$exe -v`)
end
Copy link
Member Author

Choose a reason for hiding this comment

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

@amontoison I think you need some tests like this.

In the normal tests, we can't open the executable, but we trap the error and return OTHER_ERROR to MOI so it looks as if the binary didn't find a solution, but really it crashed.

@odow
Copy link
Member Author

odow commented Jun 7, 2023

@amontoison the windows executable is still a problem

@amontoison
Copy link
Collaborator

I don't know where is the issue Oscar. :(
I should try to compile with an older CompilerSupportLibraries_jll.

@giordano
Copy link

giordano commented Jun 7, 2023

That doesn't make much sense since we do dynamic linking. But using an older gcc+binutils can make a difference

@amontoison
Copy link
Collaborator

amontoison commented Jun 7, 2023

That doesn't make much sense since we do dynamic linking.

Indeed, it is not relevant.

But using an older gcc+binutils can make a difference

The issue is that ipopt uses libgfortran and we can't use a compiler older than GCC8 if we link with libgfortran5.
What I don't understand is that is that we now have an error if we recompile the working version 300.1400.400 with exactly the same options.

@amontoison
Copy link
Collaborator

@odow Can you try the last Ipopt_jll.jl ?

test/MINLPTests/Project.toml Outdated Show resolved Hide resolved
Project.toml Outdated Show resolved Hide resolved
@odow odow mentioned this pull request Feb 20, 2024
@odow
Copy link
Member Author

odow commented Feb 20, 2024

Fixed at last

@odow
Copy link
Member Author

odow commented Feb 20, 2024

image

@odow odow merged commit 732090e into master Feb 20, 2024
13 checks passed
@odow odow deleted the od/fix-blas branch February 20, 2024 22:26
@amontoison
Copy link
Collaborator

Amazing @odow 🎉 🎉
What was the issue finally?

@odow
Copy link
Member Author

odow commented Feb 20, 2024

No idea. But you must have done something. Perhaps the openmp stuff? Or just general Julia/lbt improvements.

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

Successfully merging this pull request may close these issues.

4 participants