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

Use a name other than full for returning original array from a factorization #229

Closed
tkelman opened this issue Jul 14, 2015 · 17 comments · Fixed by JuliaLang/julia#24250
Closed
Assignees
Labels
needs decision A decision on this change is needed

Comments

@tkelman
Copy link

tkelman commented Jul 14, 2015

ref discussion in JuliaLang/julia#12137. Calling this operation full and returning anything other than a dense array from full is fishy to me. I think it's different enough to warrant a separate name, though I'm not sure what that name should be. Maybe this is an uncommon enough operation that we just force the user to know what type it came from and use convert. Or we come up with something new, ifactorize maybe? Suggestions welcome.

@andreasnoack
Copy link
Member

prod?

@jiahao
Copy link
Member

jiahao commented Jul 15, 2015

Another possibility is to bring back dense() for returning dense matrices and allow full() to return something else.

@ViralBShah
Copy link
Member

factorproduct

@andreasnoack
Copy link
Member

@ViralBShah I don't think factor should be in the method name when Factor is part of the name of the type for which the method is defined.

@Sacha0
Copy link
Member

Sacha0 commented Jun 16, 2016

collapse? condense?

@rfourquet
Copy link
Member

In the number theory package "pari", this function is named factorback (but it's not in the linear algebra context).

@StefanKarpinski
Copy link
Member

Matrix/Array

@andreasnoack
Copy link
Member

@StefanKarpinski I think we are looking for something generic and it would be weird if Matrix(CHOLMOD.Factor) -> SparseMatrixCSC.

@StefanKarpinski
Copy link
Member

That's a fair point. AbstractMatrix?

@andreasnoack
Copy link
Member

Yes. I think that could work.

@tkelman
Copy link
Author

tkelman commented Jun 16, 2016

Are factorization types already subtypes of AbstractMatrix though?

@andreasnoack
Copy link
Member

No and it even has a name #2. It wouldn't work if that was the case.

@tkelman
Copy link
Author

tkelman commented Jun 16, 2016

Ah my bad. In that case I guess it's alright - calling a constructor of an abstract type always seems a little odd to me but it should be mostly unambiguous here.

andreasnoack referenced this issue in JuliaLang/julia Jul 10, 2016
…nvert(Array, X) and add convert(AbstractArray, X) methods for Factorizations (#17066)

* Implement convert(Matrix, SparseMatrixCSC) <- convert(Array, SparseMatrixCSC) chain, and reimplement full(SparseMatrixCSC) as a synonymous child of convert(Array, SparseMatrixCSC).

* Implement convert(Vector, AbstractSparseVector) <- convert(Array, AbstractSparseVector) chain, and reimplement full(AbstractSparseVector) as a synonymous child of convert(Array, AbstractSparseVector).

* Implement convert(Matrix, Bidiagonal) <- convert(Array, Bidiagonal) chain, and reimplement full(Bidiagonal) as a synonymous child of convert(Array, Bidiagonal).

* Implement convert(AbstractMatrix, X) <- convert(AbstractArray, X) <- convert(Matrix, X) <- convert(Array, X) chains for Cholesky and CholeskyPivoted types, and reimplement full(X) for both types as synonymous children of convert(Array, X).

* Implement convert(Matrix, Diagonal) <- convert(Array, Diagonal) chain, and reimplement full(Diagonal) as a synonymous child of convert(Array, Diagonal).

* Implement convert(AbstractMatrix, Eigen) <- convert(AbstractArray, Eigen) <- convert(Matrix, Eigen) <- convert(Array, Eigen) chain, and reimplement full(Eigen) as synonymous child of convert(Array, Eigen).

* Implement convert(AbstractMatrix, Hessenberg) <- convert(AbstractArray, Hessenberg) <- convert(Matrix, Hessenberg) <- convert(Array, Hessenberg) and convert(Matrix, HessenbergQ) <- convert(Array, HessenbergQ )chains, and reimplement full(X) for both types as synonmous children of convert(Array, X).

* Implement convert(SymTridiagonal, LDLt) <- convert(AbstractMatrix, LDLt) <- convert(AbstractArray, LDLt) <- convert(Matrix, LDLt) <- convert(Array, LDLt) chain, and reimplement full(LDLt) as a synonymous child of convert(Array, LDLt).

* Implement convert(AbstractMatrix, LQ) <- convert(AbstractArray, LQ) <- convert(Matrix, LQ) <- convert(Array, LQ) and convert(Matrix, LQPackedQ) <- convert(Array, LQPackedQ) chains, and reimplement full(X) for both types as a synonymous child of convert(Array, X).

* Implement convert(AbstractMatrix, X) < convert(AbstractArray, X) <- convert(Matrix, X) <- convert(Array, X) chains for LU and LU{T,Tridiagonal{T}} types, and reimplement full(X) for both types as synonymous children of convert(Array, X).

* Implement convert(AbstractArray, Union{QR,QRCompactWY}) <- convert(AbstractMatrix, Union{QR,QRCompactWY}) <- convert(Matrix, Union{QR,QRCompactWY}) <- convert(Array, Union{QR,QRCompactWY}) and convert(Matrix, Union{QRPackedQ,QRCompactWYQ}) <- convert(Array, Union{QRPackedQ,QRCompactWYQ}) chains, and reimplement full(Union{QR,QRCompactWY}) and full(Union{QRPackedQ,QRCompactWYQ}) as synonymous children of convert(Array, Union{QR,QRCompactWY}) and convert(Array, Union{QRPackedQ,QRCompactWYQ}) respectively.

* Implement convert(AbstractMatrix, Schur) <- convert(AbstractArray, Schur) <- convert(Matrix, Schur) <- convert(Array, Schur) chain, and reimplement full(Schur) as a synonymous child of convert(Array, Schur).

* Implement convert(AbstractMatrix, SVD) <- convert(AbstractArray, SVD) <- convert(Matrix, SVD) <- convert(Array, SVD) chain, and reimplement full(SVD) as a synonymous child of convert(Array, SVD).

* Implements convert(Matrix, Symmetric) and convert(Matrix, Hermitian). Implements convert(Array, Union{Symmetric,Hermitian}) as a short child of the two preceding methods, and reimplements full(Union{Symmetric,Hermitian}) likewise.

* Implements convert(Array, AbstractTriangular) as a short child of convert(Matrix, AbstractTriangular), and reimplements full(AbstractTriangular) as a short child of convert(Array, AbstractTriangular).

* Implements convert(Array, Tridiagonal) and convert(Array, SymTridiagonal) as synonymous children of convert(Matrix, Tridiagonal) and convert(Matrix, SymTridiagonal) respectively, and reimplements full(Tridiagonal) and full(SymTridiagonal) as short children of convert(Array, Tridiagonal) and convert(Array, SymTridiagonal) respectively.
@Sacha0
Copy link
Member

Sacha0 commented Jul 21, 2016

With JuliaLang/julia#17066 merged, close?

mfasi referenced this issue in mfasi/julia Sep 5, 2016
…nvert(Array, X) and add convert(AbstractArray, X) methods for Factorizations (JuliaLang#17066)

* Implement convert(Matrix, SparseMatrixCSC) <- convert(Array, SparseMatrixCSC) chain, and reimplement full(SparseMatrixCSC) as a synonymous child of convert(Array, SparseMatrixCSC).

* Implement convert(Vector, AbstractSparseVector) <- convert(Array, AbstractSparseVector) chain, and reimplement full(AbstractSparseVector) as a synonymous child of convert(Array, AbstractSparseVector).

* Implement convert(Matrix, Bidiagonal) <- convert(Array, Bidiagonal) chain, and reimplement full(Bidiagonal) as a synonymous child of convert(Array, Bidiagonal).

* Implement convert(AbstractMatrix, X) <- convert(AbstractArray, X) <- convert(Matrix, X) <- convert(Array, X) chains for Cholesky and CholeskyPivoted types, and reimplement full(X) for both types as synonymous children of convert(Array, X).

* Implement convert(Matrix, Diagonal) <- convert(Array, Diagonal) chain, and reimplement full(Diagonal) as a synonymous child of convert(Array, Diagonal).

* Implement convert(AbstractMatrix, Eigen) <- convert(AbstractArray, Eigen) <- convert(Matrix, Eigen) <- convert(Array, Eigen) chain, and reimplement full(Eigen) as synonymous child of convert(Array, Eigen).

* Implement convert(AbstractMatrix, Hessenberg) <- convert(AbstractArray, Hessenberg) <- convert(Matrix, Hessenberg) <- convert(Array, Hessenberg) and convert(Matrix, HessenbergQ) <- convert(Array, HessenbergQ )chains, and reimplement full(X) for both types as synonmous children of convert(Array, X).

* Implement convert(SymTridiagonal, LDLt) <- convert(AbstractMatrix, LDLt) <- convert(AbstractArray, LDLt) <- convert(Matrix, LDLt) <- convert(Array, LDLt) chain, and reimplement full(LDLt) as a synonymous child of convert(Array, LDLt).

* Implement convert(AbstractMatrix, LQ) <- convert(AbstractArray, LQ) <- convert(Matrix, LQ) <- convert(Array, LQ) and convert(Matrix, LQPackedQ) <- convert(Array, LQPackedQ) chains, and reimplement full(X) for both types as a synonymous child of convert(Array, X).

* Implement convert(AbstractMatrix, X) < convert(AbstractArray, X) <- convert(Matrix, X) <- convert(Array, X) chains for LU and LU{T,Tridiagonal{T}} types, and reimplement full(X) for both types as synonymous children of convert(Array, X).

* Implement convert(AbstractArray, Union{QR,QRCompactWY}) <- convert(AbstractMatrix, Union{QR,QRCompactWY}) <- convert(Matrix, Union{QR,QRCompactWY}) <- convert(Array, Union{QR,QRCompactWY}) and convert(Matrix, Union{QRPackedQ,QRCompactWYQ}) <- convert(Array, Union{QRPackedQ,QRCompactWYQ}) chains, and reimplement full(Union{QR,QRCompactWY}) and full(Union{QRPackedQ,QRCompactWYQ}) as synonymous children of convert(Array, Union{QR,QRCompactWY}) and convert(Array, Union{QRPackedQ,QRCompactWYQ}) respectively.

* Implement convert(AbstractMatrix, Schur) <- convert(AbstractArray, Schur) <- convert(Matrix, Schur) <- convert(Array, Schur) chain, and reimplement full(Schur) as a synonymous child of convert(Array, Schur).

* Implement convert(AbstractMatrix, SVD) <- convert(AbstractArray, SVD) <- convert(Matrix, SVD) <- convert(Array, SVD) chain, and reimplement full(SVD) as a synonymous child of convert(Array, SVD).

* Implements convert(Matrix, Symmetric) and convert(Matrix, Hermitian). Implements convert(Array, Union{Symmetric,Hermitian}) as a short child of the two preceding methods, and reimplements full(Union{Symmetric,Hermitian}) likewise.

* Implements convert(Array, AbstractTriangular) as a short child of convert(Matrix, AbstractTriangular), and reimplements full(AbstractTriangular) as a short child of convert(Array, AbstractTriangular).

* Implements convert(Array, Tridiagonal) and convert(Array, SymTridiagonal) as synonymous children of convert(Matrix, Tridiagonal) and convert(Matrix, SymTridiagonal) respectively, and reimplements full(Tridiagonal) and full(SymTridiagonal) as short children of convert(Array, Tridiagonal) and convert(Array, SymTridiagonal) respectively.
@tkelman
Copy link
Author

tkelman commented Sep 14, 2016

I think JuliaLang/julia#17082 will close this with the deprecation. Right now things have been reimplemented in terms of convert, but using full for this operation isn't deprecated yet, correct?

@Sacha0
Copy link
Member

Sacha0 commented Sep 14, 2016

Correct. With #350 and JuliaLang/julia#17900 in, JuliaLang/julia#17660 should now be viable (will confirm there). JuliaLang/julia#17660 should enable reinstatement of JuliaLang/julia#17079, which should in turn enable JuliaLang/julia#17082 and close this issue. Best!

@Sacha0
Copy link
Member

Sacha0 commented Dec 15, 2016

Deferring till after 0.6 (the sparse map[!]/broadcast[!] work is higher priority). Best!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs decision A decision on this change is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants