-
-
Notifications
You must be signed in to change notification settings - Fork 213
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
Fix ldiv / rdiv + implement pinv #223
Conversation
end | ||
end | ||
|
||
@adjoint function /(A::AbstractMatrix, B::Union{Diagonal, AbstractTriangular}) |
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.
What about /
for abstract matrices? Is that still covered and I'm missing it?
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.
Yup. /
is implemented in terms of \
for abstract matrices :)
Z = A \ B | ||
return Z, function(Z̄) | ||
B̄ = A' \ Z̄ | ||
if size(A, 1) == size(A, 2) |
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.
Don't we also need that A
is nonsingular?
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 just elected to follow the structure in base found here precisely. I think base assumes that the user will ensure that A
is well-conditioned.
LGTM. If you need the adjoint/transpose fixes can you rebase or pick the commits from #216, so we can include the credit? |
Well that's uncanny. I hadn't realised #216 is a thing, and somehow we've managed to implement pretty much the same thing character-for-character... Have cherry-picked both commits (not done this before, so may have made a mess of it 😬) |
Funny, yeah they really do seem identical. Once tests pass I think this is good to go. |
Relates to #222