Skip to content

Add DI warning to AutoEnzyme #109

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add DI warning to AutoEnzyme #109

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/dense.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ Struct used to select the [Enzyme.jl](https://github.com/EnzymeAD/Enzyme.jl) bac

Defined by [ADTypes.jl](https://github.com/SciML/ADTypes.jl).

!!! warning
`AutoEnzyme` can be used by [DifferentiationInterface.jl](https://github.com/JuliaDiff/DifferentiationInterface.jl) to access a restricted subset of Enzyme.jl functionality.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like the wrong place to have it, since sciml uses ADTypes (created the package), and calls Enzyme directly

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My reasoning is that high-level users often don't make the call to DI themselves, but they do provide an AutoEnzyme (or AutoForwardDiff, etc.) object to whatever package they are calling. That's why putting a warning in ADTypes makes sense to me, because it is the entry point that users actually control about derivative computation.

I also plan on making the corresponding Enzyme warning in the DI docs more prominent. And I will let you have the final word in any Enzyme-related discussion on Discourse, Slack and other platforms, instead of piling up about what DI can or can't do. As long as users are aware they can try both options, I'm good with that. Consider this an olive branch :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah but this still feels like the wrong place for this to happen, since ADTypes often directly calls enzyme and doesn’t go through DI.

As mentioned in the other thread I’m working on trying to make the errors throw only if called within DI

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this would be a good addition for the many packages who do call Enzyme through DI, without hurting the others, but I'll leave it up to you. Feel free to revive this PR if you change your mind, I'm setting it as a draft for the time being.

If your code or one of its dependencies computes derivatives through DifferentiationInterface.jl, you might experience slowdowns or errors that would not happen with Enzyme.jl's native API.
In that case, please refer to the [Enzyme.jl documentation](https://enzymead.github.io/Enzyme.jl/stable/) for details on how to call its native API.

# Constructors

AutoEnzyme(; mode::M=nothing, function_annotation::Type{A}=Nothing)
Expand Down
Loading