-
Notifications
You must be signed in to change notification settings - Fork 2
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
Start using DifferentiationInterface #140
Start using DifferentiationInterface #140
Conversation
Hi @gdalle; many thanks for the PR!
Sure. Right now forward mode is OK for the internal AD we need. (Mostly taking gradients of functions with < 1e2 variables.) There would be good reasons to switch to reverse, though. To be tested.
Yes. Right now, defining a single default
Just have had a look at DifferentiationInterface doc: very nice mechanism. Will be very interesting to use when (i) solving an |
I made the changes, not sure why CI failed the first time but the tests pass locally (at least on 1.10). I guess it's related to your local registry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @gdalle your suggestion to change the API is indeed the good one to have a full parametrisation by a single backend. The more sophisticated alternative being to be able to specify and combine several backends (e.g. to compute second order derivatives).
Yes, in the general case you may want to let the user specify
But for problems with <100 variables, I think ForwardDiff is close to optimal in all of these settings, provided you reuse the preparation step. |
Did you figure out why CI fails? |
7440621
into
control-toolbox:differentiationinterface
Maybe the file
steps:
- uses: actions/checkout@v4
- uses: julia-actions/setup-julia@latest
with:
version: ${{ matrix.version }}
arch: ${{ matrix.arch }}
- uses: julia-actions/add-julia-registry@v2
with:
key: ${{ secrets.SSH_KEY }}
registry: control-toolbox/ct-registry
- uses: julia-actions/julia-runtest@latest
- uses: julia-actions/julia-uploadcodecov@latest
env:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} |
In the package Here is the piece of code: function rhs(h::AbstractHamiltonian)
function rhs!(dz::DCoTangent, z::CoTangent, v::Variable, t::Time)
n = size(z, 1) ÷ 2
foo(z) = h(t, z[rg(1,n)], z[rg(n+1,2n)], v)
dh = ctgradient(foo, z)
dz[1:n] = dh[n+1:2n]
dz[n+1:2n] = -dh[1:n]
end
return rhs!
end If we want to use the preparation step, should we keep track of function rhs(h::AbstractHamiltonian)
# compute preparation step here to get extras?
function rhs!(dz::DCoTangent, z::CoTangent, v::Variable, t::Time)
n = size(z, 1) ÷ 2
foo(z) = h(t, z[rg(1,n)], z[rg(n+1,2n)], v)
dh = ctgradient(foo, z) # use extras as an argument to ctgradient?
dz[1:n] = dh[n+1:2n]
dz[n+1:2n] = -dh[1:n]
end
return rhs!
end |
@ocots side note: any reason to write this
instead of
|
✅ check this WIP
✅ |
checks passed, well done @ocots 👍🏽 please document somewhere what you did to solve the CI issue (
|
The general rules of preparation are given in this section of the docs. See in particular the paragraph on reusing The trouble here is that you need the function |
I thing it is because of the scalar case. |
🙏🏽 Reads "If your package depends on private packages registered in a private registry..." Indeed another reason for the next move to the general registry 🤞🏾 |
Oh right. Again. No |
Thanks for links. I see your point. There is no possibility to differentiate a parametric function |
DifferentiationInterface was built to support 1-argument functions |
This PR is the beginning of a solution for #25. It shows how you can use DifferentiationInterface to compute derivatives in a backend-agnostic fashion.
It seems mergeable to me, but here are some more things you can do to increase performance:
backend
object(s) from the user-facing functions all the way down to the utility functions.extras
that arise from the preparation step, and if so, adjust the code to initialize them once and then pass them around.Ping @ocots @jbcaillau