-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
spezialized conj of Transpose/Adjoint #33609
spezialized conj of Transpose/Adjoint #33609
Conversation
Clever. This makes sense to me (although it took me a second). |
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.
LGTM. I don't have own experience with the lazy vs copy issue of conj
. At some point, @andreasnoack suggested to consider to introduce a lazy conj
type, in the style of Adjoint
and Transpose
. This PR kind of introduces that for a very specific situation. Everywhere else, conj
does create a copy, and people might rely on that, but I don't know how to check that. OTOH, many people who care seem to be in favor of it, so why not.
Thanks for the quick review, @dkarrasch! I've added another test case, that checks, whether we do the right thing for custom matrix types, that have a different implementation for |
Win32 failure is a no-output timeout. Is this good to go? |
I think so, from the code point of view. The question is whether someone with an actual working experience with complex linear algebra should step up and make a personal statement regarding the lazy vs. copy aspect? |
I think this is fine. We generally try to avoid copies and ask people to rely on implicit copies. |
I brought this up on Slack and it received quite positive feedback, so I put this together as a PR. The implementation is very minimal. Two things to maybe think about:
AbstractArray{<:Real}
,conj
is generally a no-op, this breaks this assumption. We still differentiate betweenAdjoint
andTranspose
for real matrices though, so I think this is fine.conj
? – Probably not worth it?I am also not quite sure, how breaking one should consider this change. It will definitely break code, that relies on
conj
returning a copy here.