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 #12153

Closed
tkelman opened this issue Jul 14, 2015 · 17 comments · Fixed by #24250
Closed

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

tkelman opened this issue Jul 14, 2015 · 17 comments · Fixed by #24250
Assignees
Labels
linear algebra Linear algebra needs decision A decision on this change is needed
Milestone

Comments

@tkelman
Copy link
Contributor

tkelman commented Jul 14, 2015

ref discussion in #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
Contributor 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 #1412. It wouldn't work if that was the case.

@tkelman
Copy link
Contributor 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 pushed a commit that referenced this issue 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 #17066 merged, close?

mfasi pushed a commit to mfasi/julia that referenced this issue Sep 5, 2016
…method defs. as convert(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.
@StefanKarpinski StefanKarpinski added this to the 0.6.0 milestone Sep 13, 2016
@tkelman
Copy link
Contributor Author

tkelman commented Sep 14, 2016

I think #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 #17684 and #17900 in, #17660 should now be viable (will confirm there). #17660 should enable reinstatement of #17079, which should in turn enable #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
linear algebra Linear algebra needs decision A decision on this change is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants