TuringLang / Turing.jl

Bayesian inference with probabilistic programming.
https://turinglang.org
MIT License
2k stars 216 forks source link

Compat issue #2251

Closed mhauru closed 3 weeks ago

mhauru commented 3 weeks ago

We have a problem with ADType/SciMLBase versions in #2237 where some tests are failing on Julia 1.7. I'll try to summarise my understanding of it below. This is partially to clarify my own thoughts, excuse me if this is a bit hard to follow, hopefully you can skip some of the details.

  1. Optimization.jl gets its OptimizationFunction type from SciMLBase. One of its constructor arguments is an AbstractADType.
  2. SciMLBase started using ADType.AbstractADType only from its v1.92.1 onwards, at v1.92.0 they still define their own AbstractADType. Until then they also had no compat entry for ADTypes.
  3. Optimization.jl dropped support for Julia 1.6, 1.7, and 1.8 in its version v3.14.1.
  4. Until v3.21 Optimization has a compat entry supporting SciMLBase all the way down to v1.79. At v3.21 the SciMLBase dependency is bumped to SciMLBase v2.16.3.
  5. SciMLBase started supporting ADTypes v1 at its v2.34, until then it was ADTypes v0.2 or lower.
  6. We dropped support for ADTypes v0.2 a few days ago.

So when installing on Julia 1.7

  1. We get Optimization.jl v3.14, because that's the latest that supports 1.7.
  2. We would need a version of SciMLBase that's compatible with both Optimization.jl v3.14, which means something in ^1.79, but also something that supports ADTypes v1, which means ^v2.34. These are clearly incompatible.
  3. But fear not, for there's always SciMLBase 1.92.0 which provides no compat bounds whatsoever for ADTypes! So that's what we get.
  4. But SciMLBase v1.92.0 doesn't even interface with ADTypes, and thus our tests fail with a "no matching method" when constructing an OptimizationFunction.

I'm not quite sure what to do about this. Can we restore support for ADTypes v0.2, which would then allow e.g. SciMLBase v1.95, and we would be fine? Or could we drop support for Julia <1.9? Something else?

devmotion commented 3 weeks ago

Doesn't help but IMO it's pretty annoying that SciML packages bumped the Julia compat in this way (even though many packages would still work, sometimes with minor modifications, on older Julia versions). Turing is not the first project where this causes problems.

The ADTypes updates also broke many packages due to incorrect compat entries...

But SciMLBase v1.92.0 doesn't even interface with ADTypes, and thus our tests fail with a "no matching method" when constructing an OptimizationFunction.

So it seems we could just use subtypes of SciMLBase.AbstractADType on older Julia versions in the tests and it would work?

Generally, could we move the Optimization code to an extension (dependent on SciMLBase?), similar to the Optim extension? Then we would be a bit less affected by incompatibilities with the SciML ecosystem and (possibly) pull in fewer SciML dependencies.

torfjelde commented 3 weeks ago

Yeah all of this is annoying :confused:

Can we restore support for ADTypes v0.2, which would then allow e.g. SciMLBase v1.95, and we would be fine?

IMO the way to go to fix this asap. The only reason why we bumped this was because https://github.com/TuringLang/Turing.jl/pull/2236 wanted Tapir.jl to be explicitly exported from Turing.jl (which seems somewhat premature IMO).

@yebai Can we drop export of Tapir.jl? It will still be available for users who want to use it through using ADTypes, so we're not dropping any functionality.

torfjelde commented 3 weeks ago

Generally, could we move the Optimization code to an extension (dependent on SciMLBase?), similar to the Optim extension? Then we would be a bit less affected by incompatibilities with the SciML ecosystem and (possibly) pull in fewer SciML dependencies.

Part of the motivation to move over to Optimization.jl was to not have to make all these extensions :confused: That way, we could get the benefit of all the different optimization packages through Optimization.jl, e.g. eventually drop the extension for Optim.jl.

devmotion commented 3 weeks ago

It would just be a single extension on SciMLBase or Optimization, which would still allow Turing to benefit from all different optimization packages through Optimization. In particular, since apart from some core default optimization functionality users have to load the respective backend anyway it seems fine (i.e., without impacting usability) to move the Optimization.jl functionality to an extension. (BTW there is also a more lightweight OptimizationBase.jl which could be an alternative to SciMLBase for an extension.)

torfjelde commented 3 weeks ago

Ah gotcha; yeah that I'm happy with:)

yebai commented 3 weeks ago

@yebai Can we drop export of Tapir.jl? It will still be available for users who want to use it through using ADTypes, so we're not dropping any functionality.

I am happy to export Tapir only for Julia 1.10 to avoid a big chunk of work before merging #2237.

Note that Tapir only supports Julia 1.10 anyway.

yebai commented 3 weeks ago

Also, I am not sure moving stuff to extensions would help avoid dependency conflicts: doesn't Julia require all weakdeps to also have compat entries?

devmotion commented 3 weeks ago

It does, but the (possible) advantage would be that by default SciMLBase/OptimizationBase/Optimization would not be installed when installing Turing, so any conflicts with (dependencies of) SciMLBase/OptimizationBase/Optimization would not matter for users that do not want to use the Optimization-dependent features of Turing.

torfjelde commented 3 weeks ago

Note that Tapir only supports Julia 1.10 anyway.

Oh okay then we should definitively do that :+1:

mhauru commented 3 weeks ago

So is the idea to reintroduce compat with ADTypes v0.2 and then have something like

if Pkg.installed()["ADTypes"] >= v"1"
    using ADTypes: AutoTapir
    export AutoTapir
end

?

mhauru commented 3 weeks ago

I have no strong opinion on whether the optimisation stuff should be an extension, I'm not yet familiar with how extensions are typically used. It would change our interface a bit though, in that currently we can export maximum_likelihood and maximum_a_posteriori and offer them to users as features that Turing has, and the user can be oblivious to the fact that they even use Optimization.jl under the hood.

yebai commented 3 weeks ago

It looks good -- I'd also add a check on Julia's version. You can also do

@static if VERSION >= v"1.10" && Pkg.installed()["ADTypes"] >= v"1"
    using ADTypes: AutoTapir
    export AutoTapir
end

EDIT: the one below might be better since it avoids calling the depreciated Pkg.installed() function:



using Compat
using ADTypes

@static if VERSION >= v"1.10" && pkgversion(ADTypes) >= v"1"
    export AutoTapir
end
yebai commented 3 weeks ago

Fixed by #2252