Basic eig(h) forward rules#245
Conversation
|
Your PR no longer requires formatting changes. Thank you for your contribution! |
|
CUDA + Mooncake here probably needs #244 |
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
|
This is getting hit with the |
lkdvos
left a comment
There was a problem hiding this comment.
Overall looks good! would you like me to add some optimization comments, or prefer me not to?
|
Feel free to suggest optimizations anytime, I again did this inefficiently
in order to keep things straight at first.
…On Mon, Jun 8, 2026 at 9:31 PM Lukas Devos ***@***.***> wrote:
***@***.**** commented on this pull request.
Overall looks good! would you like me to add some optimization comments,
or prefer me not to?
------------------------------
In ext/MatrixAlgebraKitEnzymeExt/MatrixAlgebraKitEnzymeExt.jl
<#245 (comment)>
:
> + A::Annotation{TA},
+ arg::Annotation,
alg::Const{<:MatrixAlgebraKit.AbstractAlgorithm},
- ) where {RT, TA}
+ ) where {TA, RT}
⬇️ Suggested change
- A::Annotation{TA},
- arg::Annotation,
- alg::Const{<:MatrixAlgebraKit.AbstractAlgorithm},
- ) where {RT, TA}
- ) where {TA, RT}
+ A::Annotation,
+ arg::Annotation,
+alg::Const{<:MatrixAlgebraKit.AbstractAlgorithm},
+ ) where {RT}
I don't think this is used here?
—
Reply to this email directly, view it on GitHub
<#245?email_source=notifications&email_token=AAGKJYZXX3A6V4DCPPHV3HD464H7NA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINBVGA4DCMZQHA22M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-4450813085>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGKJY53IHDP3YZW5H3XWKT464H7NAVCNFSM6AAAAACZ3TI4G2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DINJQHAYTGMBYGU>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/AAGKJY5EKLEPIRW3FBG6LTL464H7NA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINBVGA4DCMZQHA22M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG>
and Android
<https://github.com/notifications/mobile/android/AAGKJY4TJ7STEA55IQIUUG3464H7NA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINBVGA4DCMZQHA22M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>.
Download it today!
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Apply Lukas' suggestion
lkdvos
left a comment
There was a problem hiding this comment.
As a small question: do you know if the pushforwards are supposed to accumulate the results into the provided destinations, or simply writing them into it is enough?
I also noticed some slight inconsistencies in the variable names between the two pushforwards, it might be nice to make both somewhat follow the same scheme?
| end | ||
| if !iszerotangent(dV) | ||
| F = inv_safe.(transpose(diagview(D)) .- diagview(D), degeneracy_atol) | ||
| diagview(F) .= zero(eltype(F)) |
There was a problem hiding this comment.
I think this is guaranteed by degeneracy_atol > 0
Co-authored-by: Lukas Devos <ldevos98@gmail.com>
This doesn't yet handle the trunc cases at all, if anyone wants to take a swing please do feel free. I also had to turn off the forward mode test for complex numbers as there's a freedom in
Vthat's hard to account for (see https://github.com/Jutho/KrylovKit.jl/blob/master/test/ad/eigsolve.jl).