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

Do we need Requires? #269

Closed
kalmarek opened this issue Apr 13, 2023 · 6 comments
Closed

Do we need Requires? #269

kalmarek opened this issue Apr 13, 2023 · 6 comments

Comments

@kalmarek
Copy link
Collaborator

I just did an experiment locally and it turns out we could just drop Requires and remove the whole __init__() hackery:

# Copyright (c) 2014: SCS.jl contributors
#
# Use of this source code is governed by an MIT-style license that can be found
# in the LICENSE.md file or at https://opensource.org/licenses/MIT.

module SCS

import MathOptInterface
import Requires
import SCS_jll
import SparseArrays

global indirect = SCS_jll.libscsindir
global direct = SCS_jll.libscsdir

import SCS_GPU_jll
global gpuindirect = SCS_GPU_jll.libscsgpuindir

if Sys.islinux() && Sys.ARCH == :x86_64
    import SCS_MKL_jll
    import SCS_MKL_jll.MKL_jll
    global mkldirect = SCS_MKL_jll.libscsmkl
end

include("c_wrapper.jl")
include("linear_solvers/direct.jl")
include("linear_solvers/indirect.jl")
include("linear_solvers/gpu_indirect.jl")
include("linear_solvers/mkl_direct.jl")
include("MOI_wrapper/MOI_wrapper.jl")

export scs_solve

end

works just fine. The only difference is that when you try to use e.g. SCS.GpuIndirectSolver without installing CUDA first you'll get (sorry for errors in german ;)

julia> feasible_basic_problems(SCS.GpuIndirectSolver)
ERROR: could not load library "/home/kalmar/.julia/artifacts/2aaf2d3c367d9e1a41ebf36b0bf47a08f363b09b/lib/libscsgpuindir.so"
libcudart.so.11.0: Kann die Shared-Object-Datei nicht öffnen: Datei oder Verzeichnis nicht gefunden
Stacktrace:
 [1] scs_set_default_settings
   @ ~/.julia/dev/SCS/src/linear_solvers/gpu_indirect.jl:14 [inlined]
 [2] ScsSettings
   @ ~/.julia/dev/SCS/src/c_wrapper.jl:43 [inlined]
 [3] scs_solve(linear_solver::Type{SCS.GpuIndirectSolver}, m::Int64, n::Int64, A::Matrix{Float64}, P::SparseMatrixCSC{Float64, Int64}, b::Vector{Float64}, c::Vector{Float64}, z::Int64, l::Int64, bu::Vector{Float64}, bl::Vector{Float64}, q::Vector{Int64}, s::Vector{Int64}, ep::Int64, ed::Int64, p::Vector{Float64}, primal_sol::Vector{Float64}, dual_sol::Vector{Float64}, slack::Vector{Float64}; warm_start::Bool, options::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
   @ SCS ~/.julia/dev/SCS/src/c_wrapper.jl:319
 [4] scs_solve (repeats 2 times)
   @ ~/.julia/dev/SCS/src/c_wrapper.jl:278 [inlined]
 [5] feasible_basic_problems(solver::Type)
   @ Main ~/.julia/dev/SCS/test/test_problems.jl:701
 [6] top-level scope
   @ REPL[4]:1

If CUDA (e.g. through CUDA_jll) is installed but no GPU is present SCS will throw a standard CUDA error (no CUDA device found or so).


The benefits of doing this:

  • it's enough to install MKL_jll/CUDA_jll somewhere and MKLDirectSolver/GpuIndirectSolver will be operational
  • no need to load MKL_jll/CUDA_jll before SCS,
  • CUDA_jll is deprecated (and it has just a dozen new users in the last month, see https://juliahub.com/ui/Packages/CUDA_jll/nSfHz/11.8.0+0), so we will need to move away from it anyway; CUDA_Runtime_jll (8.5k new users) is the new way to go, although I haven't figured out how to use it yet.

Drawbacks of dropping Requires

  • we no longer have fine-grained control over CUDA/MKL versions loaded, or even we don't know if the jll version is used (=potentially more user-facing bugs which are hard to diagnose)
  • available_solvers are gone (but one could cook up some __init__ to bring them back).

@odow Are we happy with the status quo, should we load SCS_GPU_jll unconditionally, or should we work on a solution with CUDA_Runtime_jll?

@odow
Copy link
Member

odow commented Apr 13, 2023

remove the whole init() hackery

__init__ is required so that someone can overload the binaries.

I'm in favor of keeping Requires for now. It seems to be working, and asking people to explicitly opt into CUDA_jll (or CUDA_Runtime_jll) seems okay.

@kalmarek
Copy link
Collaborator Author

hmm? what do you mean override the binaries? The standard way I do it is to SCS_GPU_jll.dev_jll() and then replace the new library in .julia/dev/SCS_GPU_jll/override/lib; How do you do it with __init__?

@odow
Copy link
Member

odow commented Apr 13, 2023

__init__ needs to load the new path and set the global, otherwise the path gets baked into the precompiled image and the new dev_jll() path isn't used.

@kalmarek
Copy link
Collaborator Author

hmm, maybe I'm missing something: If you're in SCS project and you

pkg"dev SCS_GPU_jll"
import SCS_GPU_jll
SCS_GPU_jll.dev_jll()
using SCS

will precompile the dev-ed SCS_GPU_jll with the override set. The only situation when this goes wrong is when using SCS happens between dev and dev_jll()?

@odow
Copy link
Member

odow commented Apr 13, 2023

See jump-dev/ECOS.jl#134 #248

This can also occur if you use https://jump.dev/JuMP.jl/stable/developers/custom_solver_binaries without the pkg"dev.

@kalmarek
Copy link
Collaborator Author

I see, it's about relocability, thanks!

We will still need to think what to do about CUDA_Runtime_jll

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

No branches or pull requests

2 participants