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

ReverseDiff support in an extension #46

Open
mohamed82008 opened this issue May 23, 2023 · 7 comments
Open

ReverseDiff support in an extension #46

mohamed82008 opened this issue May 23, 2023 · 7 comments
Labels
feature New feature or request

Comments

@mohamed82008
Copy link
Collaborator

mohamed82008 commented May 23, 2023

It would be cool to make ReverseDiff work with this using the ReverseDiff.@grad macro. Examples: https://github.com/JuliaDiff/ReverseDiff.jl/blob/master/test/MacrosTests.jl#L184

@gdalle
Copy link
Collaborator

gdalle commented May 24, 2023

I thought ReverseDiff was already compatible with ChainRules?

@gdalle gdalle added the feature New feature or request label May 24, 2023
@mohamed82008
Copy link
Collaborator Author

You can "import" rules into ReverseDiff using @grad_from_chainrules. But the macro doesn't support callable structs and it needs this PR (JuliaDiff/ReverseDiff.jl#232) to support custom input types. But we will need an extension anyways because it's an opt-in system.

@gdalle
Copy link
Collaborator

gdalle commented May 27, 2023

So how do we define rules then?

@mohamed82008
Copy link
Collaborator Author

We define an internal function which takes the implicit function as its first argument. Then we define the rule on the internal function.

(f::ImplicitFunction)(args...; kwargs...) = call_implicit_function(f, args...; kwargs...)

@gdalle
Copy link
Collaborator

gdalle commented Jun 1, 2023

Do we have to move all rules to this weird scheme or is it just for ReverseDiff?
Cause redefining (f::ImplicitFunction)(args...; kwargs...) would be bad

@mohamed82008
Copy link
Collaborator Author

preferably we should move all rules to be consistent, defining that function is just an extra line of code

@gdalle
Copy link
Collaborator

gdalle commented Aug 4, 2023

The ChainRules docs says ReverseDiff doesn't support calling back into AD. That means we cannot use it for both the implicit function and the conditions.
However, once #80 is merged, we can probably use ReverseDiff for the conditions and another backend for the implicit function. Can we do the other way around? Not sure.

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

No branches or pull requests

2 participants