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

type piracy #11

Closed
stevengj opened this issue Aug 8, 2022 · 7 comments
Closed

type piracy #11

stevengj opened this issue Aug 8, 2022 · 7 comments

Comments

@stevengj
Copy link
Member

stevengj commented Aug 8, 2022

These are type piracy and should be removed: https://github.com/smataigne/SkewLinearAlgebra.jl/blob/365b0a1a4c7f07235ba0bdca07b5da8fbe30ebb8/src/hessenberg.jl#L3-L7

Just call the low-level constructor directly.

@smataigne
Copy link
Collaborator

Fixed for Hessenberg, as HessenbergQ is called using Hessenberg(not called directly by my code), I don't see how to do it for HessenbergQ without extending the definition of the construstor.

@stevengj
Copy link
Member Author

stevengj commented Aug 8, 2022

Can't you just call LA.HessenbergQ{eltype(F.factors),S,W,true}(F.uplo, F.factors, F.τ) directly?

@stevengj
Copy link
Member Author

stevengj commented Aug 8, 2022

Or is the problem that this is called internally by LinearAlgebra?

@smataigne
Copy link
Collaborator

The constructor of HessenbergQ is called internally by LinearAlgebra when the constructor of Hessenberg is called

@stevengj
Copy link
Member Author

stevengj commented Aug 9, 2022

Oh, in that case we should:

  1. Stick with type piracy for now to extend the HessenbergQ constructor
  2. Submit a patch to Julia to extend the HessenbergQ in LinearAlgebra (once that is merged we can remove our pirated constructor here using a VERSION check).

@stevengj
Copy link
Member Author

stevengj commented Aug 9, 2022

Actually, now that you have SkewHermTridiagonal, you can just use that in your HessenbergQ constructor — that will no longer be type piracy since you "own" this type.

@smataigne
Copy link
Collaborator

Thanks! I will change that now

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

No branches or pull requests

2 participants