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

Add rules for solutions to Sylvester and Lyapunov equations #384

Merged
merged 8 commits into from
Mar 2, 2021

Conversation

sethaxen
Copy link
Member

This PR adds rules for the following functions:

  • frule for LAPACK.trsyl! (solution to the Sylvester equation for (quasi)-triangular inputs)
  • frule/rrule for sylvester (solution to the Sylvester equation)
  • frule/rrule for lyap (solution to the Lyapunov equation)

All of the rules can be written in terms of the primal function itself. However, sylvester and lyap both use a Schur decomposition to solve an easier equation using LAPACK.trsyl!, so these rules then reuse that Schur decomposition. As a result, the included rule for lyap is much more efficient than the one in Zygote for StridedMatrix inputs.

FiniteDifferences only consistently works during overflow, but locally it works sometimes, which is enough to know that the scaling is correctly handled.
@codecov-io
Copy link

codecov-io commented Mar 1, 2021

Codecov Report

Merging #384 (bad4e3a) into master (99c58a9) will increase coverage by 0.07%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #384      +/-   ##
==========================================
+ Coverage   97.72%   97.80%   +0.07%     
==========================================
  Files          19       20       +1     
  Lines        1496     1547      +51     
==========================================
+ Hits         1462     1513      +51     
  Misses         34       34              
Impacted Files Coverage Δ
src/ChainRules.jl 100.00% <ø> (ø)
src/rulesets/LinearAlgebra/dense.jl 98.62% <100.00%> (+0.54%) ⬆️
src/rulesets/LinearAlgebra/lapack.jl 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 99c58a9...bad4e3a. Read the comment docs.

trans = T <: Complex ? 'C' : 'T'
∂D, scale2 = LAPACK.trsyl!(trans, trans, RA, RB, ∂Y)
∂Z = rmul!(QA * (∂D * QB'), -inv(scale2))
return NO_FIELDS, @thunk(∂Z * Ω'), @thunk(Ω' * ∂Z), @thunk(∂Z * inv(scale))
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should have a shorthand for inplaceable thunking multiplication.
Maybe @thunk should be smart in that way?

Not for this PR but just something to consider

Copy link
Member Author

Choose a reason for hiding this comment

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

That would be convenient. I haven't really put any effort into writing InplaceableThunks, since nothing supports them yet. Simple cases like this would be easy to transform but maybe could break? e.g. for the last thunk here, the macro would not know that scale is a number not a matrix. And is the case that we thunk just the multiplication common enough to make the code complexity worth it?

test/rulesets/LinearAlgebra/dense.jl Outdated Show resolved Hide resolved
test/rulesets/LinearAlgebra/lapack.jl Outdated Show resolved Hide resolved
@sethaxen sethaxen merged commit 76ef95c into JuliaDiff:master Mar 2, 2021
@sethaxen sethaxen deleted the syllyap branch March 2, 2021 20:27
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

Successfully merging this pull request may close these issues.

3 participants