-
Notifications
You must be signed in to change notification settings - Fork 37
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 svdsolve AD rule #84
Conversation
For the first commit I mostly copied the code from QuantumKitHub/PEPSKit.jl#15 and adapted it a bit to KrylovKit. As of now, this is untested but the PEPSKit version runs correctly and produces results that are consistent with the TensorKit SVD reverse-rule. However, this PR does need a bit of help to get it fully running @lkdvos. |
Great, thanks! |
Am I correct that this only works for the case where |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## ad-extension #84 +/- ##
================================================
- Coverage 77.40% 75.16% -2.25%
================================================
Files 30 31 +1
Lines 2957 2851 -106
================================================
- Hits 2289 2143 -146
- Misses 668 708 +40 ☔ View full report in Codecov by Sentry. |
Yes, I forgot to add that. I wasn't sure how KrylovKit internally handles the abstract vector types and what would be the best way to rewrite things like |
@pbrehmer , I edited this PR so it now points to the branch on the main repository. Can you still move the implementation to the extension folder? I think I can merge after |
I think this should do the job? I hope this is mergeable now. |
Ok, I pushed some updates to the ad-extension branch, but now there is a conflict again. Can I resolve it (this will also push to your master) or do you want to rebase from your side? |
Yes sure, you can go ahead and resolve it (and push), thanks! |
This PR adds a reverse-rule for
svdsolve
which implements the truncated SVD adjoint recently derived in this pre-print. See also PR #83.