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

Can DifferentiationInterface be useful for Turing? #2187

Closed
gdalle opened this issue Apr 2, 2024 · 14 comments
Closed

Can DifferentiationInterface be useful for Turing? #2187

gdalle opened this issue Apr 2, 2024 · 14 comments

Comments

@gdalle
Copy link

gdalle commented Apr 2, 2024

Hi there!
@adrhill and I recently started https://github.com/gdalle/DifferentiationInterface.jl to provide a common interface for automatic differentiation in Julia. We're currently chatting with Lux.jl, Flux.jl and Optimization.jl to see how they can benefit from it, and so my mind went to Turing.jl as another AD power user :)
DifferentiationInterface.jl only guarantees support for functions of the type f(x) = y or f!(y, x) with standard numbers or arrays in and out. Within these restrictions, we are compatible with 13 different AD backends, including the cool kids like Enzyme.jl and even the hipsters like Tapir.jl. Do you think it could come in handy?
Ping @yebai @willtebbutt

@yebai
Copy link
Member

yebai commented Apr 2, 2024

Turing's current interface to autodiff backends is based on LogDensityProblemsAD -- it is a fairly lightweight package for sharing glue code to various autodiff backends. You can take a look there to see how things might work.

@torfjelde
Copy link
Member

Looks great!

Is there a summary of how this is different from AbstractDifferentiation.jl somewhere?:)

@gdalle
Copy link
Author

gdalle commented Apr 2, 2024

I updated the summary in this issue: JuliaDiff/AbstractDifferentiation.jl#131

@Red-Portal
Copy link
Member

Red-Portal commented Jun 15, 2024

Hi, I think DifferentiationInterface could be perfectly useful for AdvancedVI. However, for that, I think it would be useful to add an interface where auxiliary information can be passed to the target function. For example, like this. This should be very useful for any statistical application that uses minibatch subsampling. Apparently, Tapir supports passing auxiliary input to the target function. So it would be nice to have a unifying interface that provides access to that. (It would also be really nice if ReverseDiff precompiled tapes provided this feature, but unfortunately doesn't.)

@gdalle
Copy link
Author

gdalle commented Jun 15, 2024

Sounds reasonable, I have opened this issue to keep track:

@yebai
Copy link
Member

yebai commented Sep 2, 2024

Closing this in favour of tpapp/LogDensityProblemsAD.jl#29; Turing will automatically use DI when tpapp/LogDensityProblemsAD.jl#29 is merged.

@yebai yebai closed this as completed Sep 2, 2024
@penelopeysm
Copy link
Member

The PR above (tpapp/LogDensityProblemsAD.jl#29) was closed, so we still aren't using DI.

#2354 though will introduce a dependency on [email protected] through OptimizationBase.

@gdalle
Copy link
Author

gdalle commented Oct 24, 2024

For the record I tried again with tpapp/LogDensityProblemsAD.jl#39, now that DI has all the features necessary, but @tpapp said he wants to redesign his library first

@tpapp
Copy link
Contributor

tpapp commented Oct 24, 2024

If Turing maintainers need this I am happy to merge the above PR as is, and we can address those changes later.

@yebai
Copy link
Member

yebai commented Oct 24, 2024

Assuming no known performance penalty exists, I think unifying the interfaces for various AutoDiff backends using DI is beneficial. DI also allows some new AutoDiff libraries (e.g. Enzyme, Mooncake) to focus more on the core functionality and less on supporting multiple interfaces.

@gdalle
Copy link
Author

gdalle commented Oct 24, 2024

For Enzyme, as stated in DI's README, there can be some performance penalties or even correctness issues that come from not DI allowing multiple arguments. Apart from that and some StaticArrays shenanigans, I believe that most performance penalties between native backends and DI should either not exist or be easily fixable. Please open an issue with an MWE whenever you spot one.

@yebai
Copy link
Member

yebai commented Oct 24, 2024

I'd suggest merging the DI PR on the understanding that @gdalle will help fix emerging (minor) performance issues, if any.

@gdalle
Copy link
Author

gdalle commented Oct 24, 2024

Note that the PR mentioned above (tpapp/LogDensityProblemsAD.jl#39) only implements the use of DI for backends that were not supported by LogDensityProblemsAD. It will be up to that package to gradually remove their existing extensions (if they want) and let DI take over for them.

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

No branches or pull requests

6 participants