-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add second forward output and autodiff backend extensions #40
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #40 +/- ##
==========================================
+ Coverage 94.82% 96.38% +1.55%
==========================================
Files 2 5 +3
Lines 58 83 +25
==========================================
+ Hits 55 80 +25
Misses 3 3
☔ View full report in Codecov by Sentry. |
We compute the Jacobian-vector product `Jv` by solving `Au = -Bv` and setting `Jv = u`. | ||
Keyword arguments are given to both `implicit.forward` and `implicit.conditions`. | ||
""" | ||
function ChainRulesCore.frule( |
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.
Why not keep this?
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.
Mostly because
- it's only useful for Diffractor at the moment
- I cannot rewrite it using AbstractDifferentiation because of add
ForwardRuleConfigBackend
(to matchReverseRuleConfigBackend
) JuliaDiff/AbstractDifferentiation.jl#80
- version: 'nightly' | ||
os: ubuntu-latest | ||
arch: x64 | ||
allow_failure: true |
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.
Added so that failures on nightly (currently due to Zygote precompilation) do not impact the "CI passing" badge
ChainRulesCore = "d360d2e6-b24c-11e9-a2a3-2a2ae2dbcce4" | ||
Krylov = "ba0b0d4f-ebba-5204-a429-3ac8c609bfb7" | ||
LinearOperators = "5c8ed15e-5a4c-59e4-a42b-c7e8811fb125" | ||
Requires = "ae029012-a4dd-5104-9daa-d747884805df" |
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.
For ForwardDiff.jl compatibility in Julia versions < 1.9
|
||
The same trick works for multiple outputs. | ||
|
||
## Constrained optimization modeling |
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.
I made the tutorial more simple on this aspect, and added details in the FAQ
@test ForwardDiff.jacobian(first ∘ implicit, X) ≈ JJ #src | ||
@test Zygote.jacobian(first ∘ implicit, X)[1] ≈ JJ #src | ||
|
||
# Skipped because of https://github.com/JuliaDiff/ChainRulesTestUtils.jl/issues/232 #src |
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.
Any idea what to do here?
|
||
#= | ||
Unsurprisingly, the Jacobian is the identity. | ||
In this instance, we could use ForwardDiff.jl directly on the solver, but it returns the wrong result (not sure why). |
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.
Still no clue why ForwardDiff fails here
When we call it as a function, it just falls back on `implicit.forward`, so unsurprisingly we get the same tuple $(y(x), z(x))$. | ||
=# | ||
|
||
(first ∘ implicit)(x) ≈ sqrt.(x) |
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.
Always using first
is cumbersome but it is the price to pay to return y, z
Dual{T}(y[i], Partials(Tuple(dy[k][i] for k in 1:N))) | ||
end | ||
|
||
z_and_dz = Dual{T}(z, Partials(Tuple(zero(z) for k in 1:N))) |
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.
This requires z
to support zero
, which is less than ideal, but I don't know how to indicate to ForwardDiff that a derivative is zero. We need something like ZeroTangent()
that is dimension-agnostic
end | ||
|
||
""" | ||
PushforwardMul!{P,N} |
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.
Replaced lazy_jacobian
with pushforward/pullback_function
due to JuliaDiff/AbstractDifferentiation.jl#83
Main changes planned:
f(x) = y, z
wherey
is the actual output andz
is any additional information used in the optimality conditions.Link to preview of new docs: https://gdalle.github.io/ImplicitDifferentiation.jl/previews/PR40/