-
-
Notifications
You must be signed in to change notification settings - Fork 212
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
Remove adjoints for LowerTriangular
and UpperTriangular
#1258
base: master
Are you sure you want to change the base?
Conversation
It seems the CR version is defined differently from the one here. Perhaps that's the source of the gradtest failures? |
I had another look at the logs, and actually the problem seems to start earlier: the pullback for |
Can we re-run the CI in order to test the hypothesis that JuliaDiff/ChainRules.jl#633 fixed this? Locally, the cholesky tests pass on Julia 1.8.0-rc3, but there are plenty of other failures: Stacktrace
|
Let's see what CI tests tell us. At least it seems there's a clear improvement locally, before 20 tests threw an error whereas it seems now only 8 fail and 1 errors. |
There are two reasons for this PR:
UpperTriangular
, so I'd like to check if it helps to remove the rules from ZygoteThere are more definitions that probably could be removed in favour of ChainRules but I wanted to keep the PR simple.