Fix full(X) where X is a factorization

[Sacha0]

In #18850 (comment), @tkelman noticed that full(X) where X is a factorization has been incorrectly calling convert(Array, X) rather than convert(AbstractArray, X) since #17066. This PR fixes that bug, restoring full's former semantics. (Not certain whether tests for this behavior would be useful given the plan to deprecate full. Perhaps making the tests in #18850 more strict [check some return types] would be the equivalent in spirit?) Best!

[tkelman]

hope this is all of them? yes, checking return types more strictly would be a good idea. thanks!

[Sacha0]

hope this is all of them?

Hope so :). (grep'd.)

yes, checking return types more strictly would be a good idea.

Is that to say that adding appropriate return type checks to #18850 as it evolves, rather than here, strikes you as a reasonable plan? Or would you prefer to add tests to full here, full's forthcoming deprecation aside? Thanks!

[Sacha0]

The errors indicate that I relaxed too many full methods from convert(Array, ...) to convert(AbstractArray, ...). In other words, full's semantics were not consistent even among the factorization types. (The breadth and number of ambiguities we've encountered in the process of migrating away from full evidences the argument for replacing full with better defined operations.)

[andreasnoack]

The errors indicate that I relaxed too many full methods from convert(Array, ...) to convert(AbstractArray, ...). In other words, full's semantics were not consistent even among the factorization types.

Some of the errors are because the Q types are not Factorizations. There is probably still inconsistencies among the old full methods were defined and your work will definately help to eliminate them. However, I still don't see that the semantics of full(T<:Factorization/Symmetric/Triangular etc) should be ill defined. At least not with my preffered definition: full(A)=mattype(A)(A) with e.g. mattype{T,S}(LU{T,S})=S. As I see it, the problem is that we decided to use explicit type conversion instead of a generic function.

[Sacha0]

Corrected version pushed. Passing linalg and sparse tests locally now.

Some of the errors are because the Q types are not Factorizations.

Yes, I shouldn't write PRs when half asleep :). Best!

[Sacha0]

However, I still don't see that the semantics of full(T<:Factorization/Symmetric/Triangular etc) should be ill defined. At least not with my preffered definition: full(A)=mattype(A)(A) with e.g. mattype{T,S}(LU{T,S})=S. As I see it, the problem is that we decided to use explicit type conversion instead of a generic function.

@andreasnoack, please see my response in #18850 (#18850 (comment)). Best!

[Sacha0]

Apart from the few factorizations with specializations for structured matrix types (LDLt for SymTridiagonal, LU for Tridiagonal), how to test that full calls convert(AbstractArray, X) rather than convert(Array, X) is not clear. The trouble is that (for matrix types in base for which the factorizations succeed) the outputs of convert(Array, X) and convert(AbstractArray, X) are usually identical. @andreasnoack, thoughts/suggestions? Thanks!

[kshyatt]

Bumping.

[tkelman]

It looks like you're working towards deprecating full pretty soon. This will be a more correct deprecation. How to best test it is an open question, but may as well not hold up this fix.

[Sacha0]

It looks like you're working towards deprecating full pretty soon.

Hopefully, but further work is blocked on this decision, so completion timescale is indeterminate. Best!