jump-dev / ECOS.jl

A Julia interface to the ECOS conic optimization solver
https://github.com/embotech/ecos
Other
41 stars 17 forks source link

Finding the artifact path on initialization improves relocatability #134

Closed ven-k closed 2 years ago

odow commented 2 years ago

What do you mean by relocatability?

codecov[bot] commented 2 years ago

Codecov Report

Merging #134 (8a9674d) into master (a436e1a) will increase coverage by 0.09%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #134      +/-   ##
==========================================
+ Coverage   79.13%   79.22%   +0.09%     
==========================================
  Files           3        3              
  Lines         230      231       +1     
==========================================
+ Hits          182      183       +1     
  Misses         48       48              
Impacted Files Coverage Δ
src/ECOS.jl 91.66% <100.00%> (+0.75%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a436e1a...8a9674d. Read the comment docs.

ven-k commented 2 years ago

We can create sysimages and relocatable-apps for quicker startup time. It is a precompiled library file. But when you generate these on machine A and want to use on some other machine B, if any directory-path is hardwired say by using @__DIR__ ... the sysimage fails to execute that function.

Here, the const ecos = ECOS_jll.libecos records the artifact-path of machine A. And when you try to use the sysimage on machine B, as the artifact-path might not always match (happens when JULIA_DEPOT_PATHs are different) sysimage fails to initialize.

odow commented 2 years ago

Ah, you're relocating the sysimage.

We should probably have a standard way of doing this across all the solvers then.

Is this (a non-const global) the recommended way of setting the paths?

Our other option is a const libecos = Ref{String}() which we will in __init__, and then calls can use libecos[]. We originally avoided doing this in order to maintain support for Julia 1.0, but a lot of solvers (like ECOS) have dropped 1.0 support.

ven-k commented 2 years ago

Our other option is a const libecos = Ref{String}()


If you are refactoring across the ecosystem, RelocatableFolders might be something you want to look at (for any @__DIR__s). I avoided that here as ecos is used in __init__. So declaring it inside it is much more convenient and also, using RelocatableFolders.@path would create a scratchspace with the entire artifact libecos.so

odow commented 2 years ago

Okay, I tried

There didn't seem to be any difference in performance.

From here: https://docs.julialang.org/en/v1/manual/calling-c-and-fortran-code/#Non-constant-Function-Specifications

image

So even though the value is not a const, it doesn't affect performance:

julia> using ECOS

julia> @code_warntype ECOS.ECOS_ver()
Variables
  #self#::Core.Const(ECOS.ECOS_ver)

Body::Cstring
1 ─ %1 = $(Expr(:foreigncall, :(Core.tuple(:ECOS_ver, ECOS.ecos)), Cstring, svec(), 0, :(:ccall)))::Cstring
└──      return %1
odow commented 2 years ago

Thanks @ven-k. Are you working for JuliaComputing? What are you using ECOS for?