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

Surprising matrix solve error message #22073

Closed
rwst opened this issue Dec 18, 2016 · 18 comments
Closed

Surprising matrix solve error message #22073

rwst opened this issue Dec 18, 2016 · 18 comments

Comments

@rwst
Copy link

rwst commented Dec 18, 2016

This cannot be right, can it?

sage: M = matrix([(3,-1,0,0),(1,1,-2,0),(0,0,0,-3)])
sage: B = matrix(QQ,3,1, [0,0,-1])
sage: M.rows()
[(3, -1, 0, 0), (1, 1, -2, 0), (0, 0, 0, -3)]
sage: B.rows()
[(0), (0), (-1)]
sage: M.nrows()
3
sage: B.nrows()
3
sage: M.solve_left(B)
...
ValueError: number of rows of self must equal number of rows of B

Component: linear algebra

Author: Alina Bucur, Renata Paramastri

Branch/Commit: 358f873

Reviewer: Kiran Kedlaya, Caitlin Lienkaemper, Travis Scrimshaw

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

@rwst rwst added this to the sage-7.5 milestone Dec 18, 2016
@sagetrac-alina
Copy link
Mannequin

sagetrac-alina mannequin commented Oct 23, 2017

@sagetrac-alina
Copy link
Mannequin

sagetrac-alina mannequin commented Oct 23, 2017

New commits:

fa579aefixed the error message for solve_left() when the number of columns is not the same on LHS and RHS

@sagetrac-alina
Copy link
Mannequin

sagetrac-alina mannequin commented Oct 23, 2017

Author: Alina, Renata

@sagetrac-alina
Copy link
Mannequin

sagetrac-alina mannequin commented Oct 23, 2017

Commit: fa579ae

@adeines
Copy link
Mannequin

adeines mannequin commented Oct 23, 2017

Changed keywords from none to sd90

@sagetrac-clienkaemper
Copy link
Mannequin

sagetrac-clienkaemper mannequin commented Oct 24, 2017

Reviewer: Caitlin Lienkaemper

@sagetrac-clienkaemper
Copy link
Mannequin

sagetrac-clienkaemper mannequin commented Oct 24, 2017

Changed keywords from sd90 to none

@sagetrac-clienkaemper
Copy link
Mannequin

sagetrac-clienkaemper mannequin commented Oct 24, 2017

Changed author from Alina, Renata to none

@tscrim
Copy link
Collaborator

tscrim commented Oct 25, 2017

comment:5

The author name is now missing (should be in "First Last" format). Also, remove all of your (added) trailing whitespace and break this long line into <80 characters long:

        If the number of columns on the left and right hand sides is different it raises the error message 'number of columns                    of self must equal number of columns of B'

Actually, I would just remove that line altogether as it doesn't add anything practical to the documentation and you have the examples.

@sagetrac-clienkaemper
Copy link
Mannequin

sagetrac-clienkaemper mannequin commented Oct 25, 2017

Author: Alina Bucur, Renata Paramastri

@sagetrac-clienkaemper
Copy link
Mannequin

sagetrac-clienkaemper mannequin commented Oct 25, 2017

comment:6

put author info that I accidentally deleted back, now w/ last names

@kedlaya
Copy link
Sponsor Contributor

kedlaya commented Oct 25, 2017

comment:7

For the record, all doctests pass on k8s.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 25, 2017

Changed commit from fa579ae to 358f873

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 25, 2017

Branch pushed to git repo; I updated commit sha1. New commits:

358f873removed trailing white spaces and long sentence

@sagetrac-alina
Copy link
Mannequin

sagetrac-alina mannequin commented Oct 25, 2017

comment:9

Removed the long sentence completely as tscrim suggested and got rid of the trailing white spaces.

@kedlaya
Copy link
Sponsor Contributor

kedlaya commented Oct 25, 2017

Changed reviewer from Caitlin Lienkaemper to Kiran Kedlaya, Caitlin Lienkaemper, Travis Scrimshaw

@kedlaya
Copy link
Sponsor Contributor

kedlaya commented Oct 25, 2017

comment:10

Looks good now (and all tests still pass). Positive review.

@vbraun
Copy link
Member

vbraun commented Oct 29, 2017

Changed branch from u/alina/surprising_matrix_solve_error_message to 358f873

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants