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

*fact(::AbstractMatrix, ::AbstractMatrix) currently missing method definitions for most special matrix types #172

Closed
jakebolewski opened this issue Feb 2, 2015 · 8 comments
Labels
docs This change adds or pertains to documentation

Comments

@jakebolewski
Copy link
Member

Bidiagonal, Diagonal, UpperTriangular, LowerTriangular, SymTridiagonal and combinations thereof are currently not implemented.

This impacts bkfact, cholfact, qrfact, schurfact, etc.

@jakebolewski jakebolewski changed the title eigfact(::AbstractMatrix, ::AbstractMatrix) is currently missing method definitions for most special matrix types *fact(::AbstractMatrix, ::AbstractMatrix) currently missing method definitions for most special matrix types Feb 2, 2015
@andreasnoack
Copy link
Member

I don't think that the *fact methods should cover all special matrix types, e.g. Bunch-Kaufman is only defined for Hermitian and symmetric matrices and only svd, eig and schur support two argument versions.

Are no method errors generally unacceptable?

@jakebolewski
Copy link
Member Author

The documentation could be updated to say this explicitly. This might be an instance where we would want to throw a deliberately unimplemented method exception (ref JuliaLang/julia#7512) to give better context why these are undefined.

If these methods are restricted to dense-strided matrices and some subset of the special matrix types perhaps the argument types should be restricted to reflect this.

For instance, eigfact is defined as:

eigfact{TA,TB}(A::AbstractArray{TA,2},B::AbstractArray{TB,2}) at linalg/factorization.jl:566

so I would expect it to work for any two arguments <: AbstractMatrix, or at least all those defined in Base.LinAlg.

@andreasnoack
Copy link
Member

I don't think that is a general design rule in base. Many methods have signatures that are way too loose then, e.g. it would hard to accept signatures with Any.

The idea in linalg is to restrict the signature as much as necessary and not more, but I don't claim that this has been followed strictly.

The method in line 566 is a promotion method to be used whenever the two arguments don't have the same element type. It is useful because the both the eigfact(StridedMatrix{T}, StridedMatrix{T}) and eigfact(Hermitian{T},Hermitian{T}) methods can use the same promotion method.

@jakebolewski
Copy link
Member Author

isposdef is also undefined for most special matrix types.

@tkelman
Copy link

tkelman commented Feb 3, 2015

If these methods are restricted to dense-strided matrices and some subset of the special matrix types perhaps the argument types should be restricted to reflect this.

That could get annoying quickly for people trying to extend these types. The special matrix types are already not very cohesively designed or extensible. These particular missing methods are a subset of the larger problem #136

@andreasnoack andreasnoack added the docs This change adds or pertains to documentation label Sep 14, 2016
@andreasnoack
Copy link
Member

The doc strings seem state when special matrices are supported. I think that is the best way to handle this. If some of the doc strings are not complete, we can open individual issues.

@tkelman
Copy link

tkelman commented Sep 14, 2016

There are unimplemented cases here that could and should be implemented. If not here, we should track those somewhere.

@tkelman
Copy link

tkelman commented Sep 14, 2016

cc @kshyatt who may have similar issues open

edit: found it, looks like #253 mostly overlaps with this

@KristofferC KristofferC transferred this issue from JuliaLang/julia Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs This change adds or pertains to documentation
Projects
None yet
Development

No branches or pull requests

3 participants