Make matrix factorizations immutable and cached

ec97c818-c7da-4a8b-aa6b-7b49474243a2 commented 14 years ago

The attached patch (based on 4.3.rc0):

changes Cholesky (all/most matrices) and SVD, QR and LU factorizations (double_dense only) so that the returned resulting matrices are immutable
adds caching for Cholesky, SVD and QR
adds pickle-able caching for LU (and it is likely there was a a bug with pickling/unpickling a matrix with a cached factorization which this patch fixes...)
improves doctests for SVD and QR (I wanted to more easily check that my changes didn't cause regressions...)
adds methods zero_at and round to dense double matrices (used in said doctests)

I hope the doctest improvements can be accepted as part of the patch even if I didn't bother to split it up.

Note that when dealing with matrix factorization doctesting, just avoiding 0 in the input goes very far with creating readable tests.

Component: linear algebra

Author: Dag Sverre Seljebotn

Reviewer: William Stein

Merged: sage-4.3.1.alpha0

Issue created by migration from https://trac.sagemath.org/ticket/7728

ec97c818-c7da-4a8b-aa6b-7b49474243a2 commented 14 years ago

Attachment: trac_7728-immutablefactors.patch.gz

ec97c818-c7da-4a8b-aa6b-7b49474243a2 commented 14 years ago

comment:2

OK, this likely need some more docs about the caching aspect...

ec97c818-c7da-4a8b-aa6b-7b49474243a2 commented 14 years ago

comment:3

Attachment: trac_7728-immutablefactors.2.patch.gz

Last attachment contains the same changes + a little more docs.

LU was previously cached, so this just makes things more consistent by caching all decompositions -- easier to remember.

I also plan to make use of the caching if I get around to fixing #4932, see my comment there, short story: solve_left should be able to use the results of a call to LU(), which makes caching a lot more important.

williamstein commented 14 years ago

comment:4

Bravo, this is an awesome patch! Positive review.

Comment for future work by somebody. I don't like this:

987             U, S, V -- immutable matrices such that $A = U*S*V.conj().transpose()$ 
988                        where U and V are orthogonal and S is zero off of the diagonal.

It's not your fault -- it was like that before. But it is wrong in so many ways wrt to Sphinx (e.g., dollar signs? V.conj().transpose() in math mode? variables should be listed separately, etc.

aghitza commented 14 years ago

comment:5

Replying to @williamstein:

It's not your fault -- it was like that before. But it is wrong in so many ways wrt to Sphinx (e.g., dollar signs? V.conj().transpose() in math mode? variables should be listed separately, etc.

Just a small comment: I believe that we can now use dollar signs for math input in Sphinx, so that shouldn't be a problem.

mwhansen commented 14 years ago

Reviewer: William Stein

mwhansen commented 14 years ago

Merged: sage-4.3.1.alpha0

sagemath / sage

Make matrix factorizations immutable and cached #7728