Skip to content
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 * for Adjoint of Givens #41190

Closed
wants to merge 1 commit into from
Closed

Remove * for Adjoint of Givens #41190

wants to merge 1 commit into from

Conversation

JeffFessler
Copy link
Contributor

This one also caused a method ambiguity for me so I wonder if it is also obsolete for the same reasons as the other ones in this PR?
There are a dozen lines more at the end of the file that look dubious too...

This one also caused a method ambiguity for me so I wonder if it is also obsolete for the same reasons as the other ones in this PR?
There are a dozen lines more at the end of the file that look dubious too...
@dkarrasch
Copy link
Member

I believe this one is necessary, because in line 58 we have

adjoint(R::Rotation) = Adjoint(R)

and Rotation <: AbstractRotation.

@JeffFessler
Copy link
Contributor Author

Apologies - I see now that the real "issue" here for me is that an AbstractRotation is not actually an AbstractMatrix and it does not have a size so currently there is no way to compose it with a LinearMap or LinearMapAA. It reminds me of JuliaLinearAlgebra/LinearMaps.jl#118

So I will resolve my method ambiguity for this one by throwing an error. I'll close this PR. Thanks!

@dkarrasch
Copy link
Member

No worries! I took the opportunity to start hunting for more of these spurious methods with types that cannot be constructed from calling adjoint or transpose.

@JeffFessler JeffFessler deleted the patch-1 branch June 11, 2021 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants