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

Replace AbstractDifferentiation with DifferentiationInterface #20

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gdalle
Copy link

@gdalle gdalle commented Jul 30, 2024

Fix #19

I wasn't able to run the tests because Soss hasn't been updated in 2 years, and has compat conflicts with DifferentiationInterface. The first one I ran into is FillArrays having released a v1 which Soss didn't know about.

I guess we may need to ask @csherrer to take a look at the CompatHelper PRs?

@marius311
Copy link
Owner

Many thanks, looking. I think we could drop Soss, as much as I like how fast/clean it is.

@gdalle
Copy link
Author

gdalle commented Jul 30, 2024

I don't want to be the reason for that though

@marius311
Copy link
Owner

Before even Soss which could be dropped, on Julia 1.9 I run into

https://github.com/gdalle/DifferentiationInterface.jl/blob/0f7957d4c636722b0629096f0d4fc9e5009dced3/DifferentiationInterface/Project.toml#L55

https://github.com/JuliaDiff/FiniteDiff.jl/blob/27f31db7e4192e086a07c3fafe6394fa05214bc6/Project.toml#L30

which makes DI + FiniteDiff only installable on Julia 1.10+. But I don't see an easy way to drop FiniteDiff (which is a transitive dep of something I def need, Optim -> NLSolverBase -> FiniteDiff).

So it would seem this PR has to make MuseInference 1.10+ unless Im mistaken? (which I don't necessarily want to do)

@gdalle
Copy link
Author

gdalle commented Jul 31, 2024

Since I started developing DI fairly recently, the compat bounds reflect the latest versions at the time, which are more recent than what you have. And the SciML ecosystem (including FiniteDiff) has been dropping Julia 1.6 faster than other packages.
In this instance, the compat bound v2.23.1 is important because DI caught a bad bug in FiniteDiff, which was then fixed in JuliaDiff/FiniteDiff.jl#186, the fix being part of https://github.com/JuliaDiff/FiniteDiff.jl/releases/tag/v2.23.1.

@marius311
Copy link
Owner

marius311 commented Jul 31, 2024

That makes sense. Maybe this is a rabbit hole you dont feel like going down, and it could well not be the only thing, but perhaps below was too aggressive of a julia version bump? Couldnt find mention anywhere of why its needed. This isnt a MuseInference thing, anything DI+FiniteDiff in the same project is going to have to be 1.10+.

JuliaDiff/FiniteDiff.jl@8ee5d7c

@gdalle
Copy link
Author

gdalle commented Jul 31, 2024

I don't maintain FiniteDiff, despite it being in the JuliaDiff orgnaization it is mostly a SciML thing, and SciML has been dropping 1.6 like a hot potato. The right person to ask about this would be @ChrisRackauckas

@gdalle
Copy link
Author

gdalle commented Jul 31, 2024

In any case, 1.10 will be the next LTS as soon as 1.11 comes out, so 1.6 should be droppable everywhere in a matter of months, if not weeks

@marius311
Copy link
Owner

marius311 commented Jul 31, 2024

Note this is about 1.9, not just 1.6, but in any case, I'll weight if I'm ready to drop 1.9.

@gdalle
Copy link
Author

gdalle commented Oct 1, 2024

Hey @marius311, just checking in on my old PRs. Do you still want to explore the switch to DI?

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

Successfully merging this pull request may close these issues.

Switch from AbstractDifferentiation to DifferentiationInterface?
2 participants