Opened 8 years ago

Closed 14 months ago

#14087 closed defect (duplicate)

inconsistent input handling of backslash operator of matrices

Reported by: ppurka Owned by: jason, was
Priority: trivial Milestone: sage-duplicate/invalid/wontfix
Component: linear algebra Keywords:
Cc: rbeezer, kcrisman Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

From this ask.sagemath question:

sage: A = matrix(QQ, 3, [1,2,4,2,3,1,0,1,2])
sage: B = matrix(QQ, 3, 2, [1,7,5,2,1,3])
sage: A\B
[  -1    1]
[13/5 -3/5]
[-4/5  9/5]
sage: A = matrix(RDF, 3, [1,2,4,2,3,1,0,1,2])
sage: B = matrix(RDF, 3, 2, [1,7,5,2,1,3])
sage: A\B
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
...
TypeError: vector of constants over Real Double Field incompatible with matrix over Real Double Field

Paraphrasing from my reply there:

The problem arises because when A is a matrix over QQ and when A is a matrix over RDF, you will find that the former allows B to be a matrix, but the latter does not. Only vectors are supported/implemented for RDF fields. As such it is not a bug, but I think this should raise a NotImplementedError when a matrix B is used over RDF A.

Attachments (1)

trac_14087-matrix.patch (2.0 KB) - added by jason 8 years ago.

Download all attachments as: .zip

Change History (13)

Changed 8 years ago by jason

comment:1 Changed 8 years ago by jason

The problem is that there is too much and too restrictive "error" handling, when really no error is necessary. I've attached a preliminary patch, but lots of the doctests will fail that check the error messages. Also, 0-column/row matrices probably aren't handled nicely.

comment:2 Changed 8 years ago by ppurka

Out of all the doctests in the sage/matrix directory only 6 of them failed, all in matrix_double_dense.pyx. Let's allow the patchbot to run the doctests, and then we can fix any other failing doctests. The current patch seems good enough since there is apparently no need to solve selectively for vectors or matrices - scipy seems to handle both just fine.

I think the try: except: statement should remain. It seems to give a decent error message when we try to solve linear equations over a matrix which is over RDF, but a vector which is over some incompatible field like a finite field. In its absence an absurd implementation-specific error message is returned:

   AttributeError: 'sage.modules.free_module_element.FreeModuleElement_generic_dense' object has no attribute 'numpy'

I am curious though - how does vector(scipy_output) or matrix(scipy_output) work? How does it know which field or ring to convert to, since we are not specifying the ring explicitly?

comment:3 Changed 8 years ago by jason

numpy arrays store a data type, and matrix() or vector() look at that datatype.

comment:4 Changed 8 years ago by jason

I agree that some of the error handling should be put back in. The patch is more to get someone started in the right direction.

comment:5 Changed 8 years ago by ppurka

Is the patchbot down? I haven't seen any activity or testing even after I kicked the patchbot a day or two ago.

comment:6 Changed 8 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:7 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:8 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:9 Changed 7 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:10 Changed 14 months ago by gh-mwageringel

  • Milestone changed from sage-6.4 to sage-duplicate/invalid/wontfix
  • Priority changed from major to trivial
  • Status changed from new to needs_review

Duplicate of #17405.

comment:11 Changed 14 months ago by kcrisman

  • Status changed from needs_review to positive_review

Good catch.

comment:12 Changed 14 months ago by chapoton

  • Resolution set to duplicate
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.