sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.47k stars 486 forks source link

coherent handling of trivial matrices (depend on #5244, #5242). #5256

Closed hivert closed 15 years ago

hivert commented 15 years ago

There where a lot of inconsistency and bugs in the handling of trivial matrices. The following patch aims to solve these and to check systematicly the coherence. Here is a selection of weirdness:

sage: m = matrix(SR, 1,1, [1])
sage: m.inverse()
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)

sage: m = matrix(RDF, 0,2)
sage: m.inverse()
[]
sage: m = matrix(RDF, 1,1)
sage: m.inverse()
---------------------------------------------------------------------------
LinAlgError                               Traceback (most recent call last)

whereas

sage: m = matrix(QQ, 1,1)
sage: m.inverse()
---------------------------------------------------------------------------
ZeroDivisionError                         Traceback (most recent call last)

Aside rewriting some error messages, changing some exception and working around several bug in particular in maxima's handling of matrix over SR, the main contribution of this patch lies in the function test_trivial_matrices_inverse in sage/matrix/matrix_space.py and its associated doctests. Trough a bunch of assertions this function indirectly checks the behavior of matrix spaces. Any new implementation of a kind of matrices should be checked be this function.

Patch Author: Florent Hivert

CC: @sagetrac-sage-combinat

Component: linear algebra

Keywords: matrices, invert, determinant

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

jasongrout commented 15 years ago
comment:2

I'm really happy you did all this. I'll look at this soon, unless someone else gets to it before me.

It'd be great to have a system-wide function that tested different Sage types for consistency on things like this. That way, all someone would have to remember to do is add their new sage type to a doctest like:

sage: check_consistency(MY_NEW_TYPE)
True

That function would automatically call things like the function in this patch and other functions for vectors, polynomials, etc.

hivert commented 15 years ago
comment:3

In his category framework, Nicolas Thiery wrote a very handy feature that allows one to add some plug in function to test properties on a parent object. For example in the category of groups there are among other the following methods (some are inherited from higher categories):

hivert commented 15 years ago

New version with a corrected typo (thanks Jason)

malb commented 15 years ago
comment:4

Attachment: empty_matrix_inverse-fh.patch.gz

Review patch looks good except

i.e. all issues are trivial.

I didn't run doctests yet, will do now.

85eec1a4-3d04-4b4d-b711-d4db03337c41 commented 15 years ago
comment:5

I have doctested this patch on top of #5242 and #5244 in my current Sage 3.3.rc1 merge tree and:

All tests passed!

Cheers,

Michael

hivert commented 15 years ago
comment:6

Replying to @malb: Done ! See the new patch.

Note that I currently didn't had time to check it. It's currently being done on my machine but it takes times. I only change docs from the first version but who knows...

hivert commented 15 years ago

Reupped after malb request on irc.

mwhansen commented 15 years ago
comment:7

Attachment: trivial_matrices_inverse-5256-submitted.patch.gz

Positive review on trivial_matrices_inverse-5256-submitted.patch.

mwhansen commented 15 years ago
comment:8

Note that there is #5274 for the TODO/FIXME.

85eec1a4-3d04-4b4d-b711-d4db03337c41 commented 15 years ago
comment:9

Merged trivial_matrices_inverse-5256-submitted.patch in Sage 3.3.rc1.

Cheers,

Michael