Make MathOptInterface.jl a weak dependency

[theogf]

MOI adds a very large loading time to Optim.jl although it is not always necessary see : jump-dev/MathOptInterface.jl#2442

This PR makes MOI.jl a weak dependency, this is the difference of time for precompiling the package:

master:

julia> @time Pkg.precompile("Optim")
Precompiling Optim
  2 dependencies successfully precompiled in 112 seconds. 53 already precompiled.
113.622423 seconds (776.52 k allocations: 56.027 MiB, 0.02% gc time, 1.25% compilation time: 92% of which was recompilation)
julia> @time using Optim
  3.414563 seconds (3.74 M allocations: 231.737 MiB, 3.75% gc time, 0.28% compilation time)

This branch

julia> @time Pkg.precompile("Optim")
  4.763922 seconds (2.90 M allocations: 213.363 MiB, 1.90% gc time, 91.22% compilation time: 92% of which was recompilation)

julia> @time using Optim
  0.401095 seconds (451.49 k allocations: 30.527 MiB, 6.22% gc time, 2.45% compilation time)

For retro-compatibility I used Requires.jl as suggested in the docs.

One important change is that I could not create a reference to Optim.Optimizer <: MOI.AbstractOptimizer in Optim, so I resorted to make a constructor function moi_optimizer() that is then defined in OptimMOIExt.jl.

[theogf]

Bump

[MilesCranmer]

+1, this looks to be by far the largest contributor to load time in SymbolicRegression.jl, by far, even though I don't use MOI at all –

    322.5 ms  MathOptInterface
     79.3 ms  MutableArithmetics
     65.1 ms  DynamicQuantities
     47.5 ms  DynamicExpressions 25.11% compilation time
     29.5 ms  FillArrays
     23.1 ms  ForwardDiff
     15.8 ms  SymbolicRegression
     14.6 ms  SpecialFunctions
     14.5 ms  CompilerSupportLibraries_jll 37.01% compilation time (47% recompilation)
      8.2 ms  StatsBase
      6.9 ms  Optim
      5.4 ms  MacroTools
      3.3 ms  OpenSpecFun_jll 52.55% compilation time
      3.2 ms  IrrationalConstants
      3.2 ms  LossFunctions
      2.9 ms  MLJModelInterface
      2.7 ms  Missings
      2.2 ms  NLSolversBase
      2.1 ms  FillArrays → FillArraysSparseArraysExt
      2.0 ms  DynamicExpressions → DynamicExpressionsOptimExt
      2.0 ms  OpenLibm_jll
      2.0 ms  Zlib_jll
      1.7 ms  LineSearches
      1.5 ms  TranscodingStreams
      1.2 ms  ArrayInterface
      1.1 ms  Bzip2_jll
      1.1 ms  DocStringExtensions
      1.1 ms  FiniteDiff
      1.0 ms  CodecZlib
      1.0 ms  DiffResults
      1.0 ms  ProgressBars
      0.9 ms  Requires
      0.8 ms  DiffRules
      0.8 ms  SortingAlgorithms
      0.8 ms  StaticArraysCore
      0.7 ms  CodecBzip2
      0.7 ms  CommonSubexpressions
      0.7 ms  DataAPI
      0.7 ms  Parameters
      0.7 ms  PositiveFactorizations
      0.7 ms  ScientificTypesBase
      0.6 ms  ArrayInterface → ArrayInterfaceStaticArraysCoreExt
      0.6 ms  FillArrays → FillArraysStatisticsExt
      0.6 ms  LogExpFunctions
      0.6 ms  StatisticalTraits
      0.6 ms  StatsAPI
      0.5 ms  DynamicQuantities → DynamicQuantitiesLinearAlgebraExt
      0.5 ms  NaNMath
      0.5 ms  PackageExtensionCompat
      0.5 ms  Reexport
      0.5 ms  SuiteSparse
      0.5 ms  TranscodingStreams → TestExt
      0.5 ms  Tricks
      0.5 ms  UnPack

[MilesCranmer]

@theogf this is currently not compatible with Julia 1.8 as package extensions weren't added yet – I would recommend adding https://github.com/cjdoris/PackageExtensionCompat.jl to manage this.

[odow]

Instead of Requires, you could do https://github.com/JuliaOpt/NLopt.jl/blob/301b759ce9221514e7508081e5be2ba0cc11e03c/src/NLopt.jl#L633-L641

@blegat should take a look at this.

I think this would also be a breaking change, but it's reasonable to call this a bug-fix. It shouldn't affect too many people.

[theogf]

@MilesCranmer I followed your suggestion and replace Requires by PackageExtensionCompat

[blegat]

You can keep the name Optim.Optimizer see for instance what we did for NLopt

[theogf]

You can keep the name Optim.Optimizer see for instance what we did for NLopt

Ah you are using global, interesting!

[theogf]

@MilesCranmer The solution in NLopt would mean switching back to the Requires solution as it needs more fine tuning, I guess keeping the consistency with the previous version is more important?
Note that under the hood, PackageExtensionCompat is just using Requires.

[blegat]

I guess keeping the consistency with the previous version is more important?

Yes, it's best to avoid having a breaking change. This naming is also a convention followed by all JuMP solvers.

[MilesCranmer]

Note that under the hood, PackageExtensionCompat is just using Requires.

Yeah this is why I suggested removing the Requires line, since you aren't using it here. Therefore on Julia 1.9+, Requires.jl won't be loaded unnecessarily.

[MilesCranmer]

What @odow is suggesting is not incompatible with PackageExtensionCompat; you just need to define Optimizer in the main codebase so it is exportable; then overload it in the extension. This is true regardless of whether you use plain Requires or PackageExtensionCompat. But if you use Requires you need to do a bunch of other stuff manually to keep compatibility for Julia < 1.9: https://discourse.julialang.org/t/package-extensions-for-julia-1-9/93397/5. Easier (and less error prone) to just let PackageExtensionCompat handle this for you.

[theogf]

Ok I somehow managed. I tested on 1.8 + 1.10 and it seems to work.
There was unfortunately no setglobal! in v < 1.9 and there is no Compat.jl equivalent for it so I had to use @eval.

[blegat]

Thanks! Yes, it's a bit delicate to get this working for both Julia v1.6 and v1.10 ^^

[MilesCranmer]

It allowed me to define Optim.Optimizer as a const for v1.8-

Then I would do the following:

if VERSION >= v"1.9.0-DEV.0"
    @eval const Optimizer = OptimMOIExt.Optimizer
else
    @eval Optimizer = OptimMOIExt.Optimizer
end

[theogf]

It allowed me to define Optim.Optimizer as a const for v1.8-

Then I would do the following:

if VERSION >= v"1.9.0-DEV.0"
    @eval const Optimizer = OptimMOIExt.Optimizer
else
    @eval Optimizer = OptimMOIExt.Optimizer
end

I'd rather avoid using @eval if possible (could not find another solution for <v1.9).

[MilesCranmer]

What's wrong with an @eval? It's all static expressions here

[ericphanson]

Since it's already a hard dep, I think it makes sense to move to a weak dep only on 1.9+, as was done in NLopt. Then Requires is not needed at all (not even via PackageExtensionCompat).

[MilesCranmer]

Since it's already a hard dep, I think it makes sense to move to a weak dep only on 1.9+, as was done in NLopt. Then Requires is not needed at all (not even via PackageExtensionCompat).

Not sure it’s possible to have a conditional weak dependency? Note that NLopt has it as a hard dependency; it still gets installed. If you want it to be a weak dependency (i.e., not forced to be installed) and to also be compatible with <1.9, Requires seems to be required. (I would prefer to have make my downstream users install and precompile a library if it’s not used)

PackageExtensionCompat is a no-op on 1.9+ so I think it’s fine.

[blegat]

Note that NLopt has it as a hard dependency; it still gets installed. If you want it to be a weak dependency (i.e., not forced to be installed) and to also be compatible with <1.9

IIUC, when you have a package both as hard and weak dependency then it's interpreted as hard dependency on Julia < 1.9 and as weak dependency on Julia >= 1.9. This made it so that Julia v1.9 would ignore a hard dependency when it's also weak precisely to allow this use case

[MilesCranmer]

Oh, weird. What is the purpose of the [weakdeps] in that case?

[pkofod]

If there's no good way to do this, we have to just accept the large load time on old versions of Julia I suppose. Thanks for working on this. I have a deadline early next week and have been away for that reason. I will review this as soon as I'm back. I don't know if you have time to look at the discussion @KristofferC but I trust your judgement on the best course of action here :)

[ericphanson]

Btw I am referring to this strategy: https://pkgdocs.julialang.org/v1/creating-packages/#Transition-from-normal-dependency-to-extension

It is very simple and commonly used since it does not affect older usage at all, and does not need Requires or other compatibility packages.

Oh, weird. What is the purpose of the [weakdeps] in that case?

So it works as a weak dep on 1.9+. The weird thing is it’s duplicated as a “dependency” as well, so older Pkg will continue to work. Newer Pkg will see it is both a weak dep and a dep and treat it like a weak dep.

[MilesCranmer]

Got it, thanks! Okay I'm on board with that. I would personally prefer using PackageExtensionCompat so that old Julia versions can also have short load times (many of my downstream users have cluster-installed Julia pinned to an older version; not everybody is always up-to-date, especially less experienced users who are less patient about load time), but no strong feelings.

[theogf]

I am not sure what the conclusion is here 😅

Should I implement the solution proposed by @blegat and @ericphanson ?

[MilesCranmer]

Well, Requires.jl has a 0.5ms load time on Julia 1.9+, and in using it, we save a 300ms load time on Julia <1.9. Seems like an obvious solution to use PackageExtensionCompat. i.e., go with the current PR. But the others might disagree on the basis of minimizing dependencies. I think for massive gains like this it's worth it though!

I would also add the

if VERSION >= v"1.9.0-DEV.0"
    @eval const Optimizer = OptimMOIExt.Optimizer
else
    @eval Optimizer = OptimMOIExt.Optimizer  # Or other way to get `const`?
end

as mentioned earlier.

[theogf]

I unified it as :

function __init__()
    @static if VERSION >= v"1.9"
        @eval Optim begin
            OptimMOIExt = Base.get_extension(@__MODULE__, :OptimMOIExt)
            const Optimizer = OptimMOIExt.Optimizer
        end
    else
        @eval Optim begin
            using .OptimMOIExt
            const Optimizer = OptimMOIExt.Optimizer
        end
    end
end

[MilesCranmer]

LGTM. But you also need to add MathOptInterface to targets.test in Project.toml, otherwise it won't install it during testing. (Did you run the tests and verify they pass?)

As we can see, the load times are more than 50% reduction of where they were before!

• Julia v1.10: 0.95s -> 0.35s
• Julia v1.6: 5.70s -> 2.57s

PackageExtensionCompat.jl is only a 0.5ms load time on v1.10, which is literally less than the measurement noise:

julia> @time_imports using Optim
      0.6 ms  Compat
      0.4 ms  Compat → CompatLinearAlgebraExt
               ┌ 3.8 ms SuiteSparse_jll.__init__()
     19.3 ms  SuiteSparse_jll 76.11% compilation time
               ┌ 4.7 ms SparseArrays.CHOLMOD.__init__() 83.53% compilation time
    132.9 ms  SparseArrays 2.97% compilation time
      0.4 ms  SuiteSparse
      1.1 ms  ArrayInterface
               ┌ 0.0 ms Requires.__init__()
      0.6 ms  Requires
      1.0 ms  FiniteDiff
      3.2 ms  IrrationalConstants
      0.8 ms  DiffRules
      0.9 ms  StaticArraysCore
      0.7 ms  ArrayInterface → ArrayInterfaceStaticArraysCoreExt
      0.9 ms  DiffResults
      8.1 ms  Preferences
               ┌ 0.8 ms OpenLibm_jll.__init__()
      1.6 ms  OpenLibm_jll
      0.6 ms  NaNMath
               ┌ 0.0 ms DocStringExtensions.__init__()
      1.3 ms  DocStringExtensions
      0.6 ms  LogExpFunctions
      0.5 ms  JLLWrappers
               ┌ 12.3 ms CompilerSupportLibraries_jll.__init__() 27.76% compilation time
     13.0 ms  CompilerSupportLibraries_jll 26.28% compilation time
               ┌ 2.8 ms OpenSpecFun_jll.__init__() 76.63% compilation time
      3.7 ms  OpenSpecFun_jll 57.06% compilation time
      5.5 ms  SpecialFunctions
      6.3 ms  MacroTools
      0.6 ms  CommonSubexpressions
     25.5 ms  ForwardDiff
               ┌ 0.1 ms Distributed.__init__()
     10.0 ms  Distributed
      2.1 ms  NLSolversBase
      0.6 ms  PositiveFactorizations
      2.8 ms  OrderedCollections
      0.4 ms  UnPack
      0.6 ms  Parameters
      1.5 ms  LineSearches
     32.7 ms  FillArrays 36.50% compilation time
      2.2 ms  FillArrays → FillArraysSparseArraysExt
      0.5 ms  PackageExtensionCompat
      0.8 ms  DataAPI
     19.3 ms  DataStructures
      1.3 ms  SortingAlgorithms
      2.9 ms  Missings
      0.9 ms  Statistics
      0.5 ms  FillArrays → FillArraysStatisticsExt
      0.7 ms  StatsAPI
      8.7 ms  StatsBase
               ┌ 0.0 ms Optim.__init__()
      5.2 ms  Optim

for a total load time of 350ms.

[KristofferC]

🎉

[theogf]

Did you run the tests and verify they pass?

I did try locally but not via ] test so it was missing. I was hoping for the CI workflows to run for testing all setups more easily 😅

But you're right it was missing.

[pkofod]

🎉

thanks for taking a look!

[blegat]

@pkofod Can you approve the CI so that it runs ?

[pkofod]

@pkofod Can you approve the CI so that it runs ?

done. Let's see :)

[codecov]

Codecov Report

Attention: Patch coverage is 50.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 85.43%. Comparing base (78ab1f4) to head (0df0cf1).

Files	Patch %	Lines
src/Optim.jl	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1081      +/-   ##
==========================================
+ Coverage   84.85%   85.43%   +0.58%     
==========================================
  Files          46       45       -1     
  Lines        3480     3276     -204     
==========================================
- Hits         2953     2799     -154     
+ Misses        527      477      -50     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[theogf]

Nice, everything is passing. Do you want me to bump the version on this MR or will you do it after the merge?
Should it be 1.10.0 or 1.9.3 ? (I think it's not worth the breakage to tag 2.0?

[MilesCranmer]

I think it could be 1.9.3 since the MOI code was just a wrapper; not usable unless someone had already loaded MOI elsewhere.

[andreasnoack]

Please see #1087