Skip to content
This repository was archived by the owner on Dec 10, 2018. It is now read-only.

Return ErrSingular for singular matrix solves#32

Merged
kortschak merged 2 commits intomasterfrom
solve
Nov 3, 2014
Merged

Return ErrSingular for singular matrix solves#32
kortschak merged 2 commits intomasterfrom
solve

Conversation

@kortschak
Copy link
Copy Markdown
Member

Fixes issue #30.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.0%) when pulling 2409b42 on solve into 2a243e3 on master.

@btracey
Copy link
Copy Markdown
Member

btracey commented Nov 3, 2014

Why do LQ and QR panic if not full rank? I see that there is a possibility to check if not full rank, just wondering what the rule is.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of the special casing in solve, it would be nice to have the m > n and m < n cases as well.

@kortschak
Copy link
Copy Markdown
Member Author

@jonlawlor
Copy link
Copy Markdown
Contributor

Does https://github.com/gonum/matrix/blob/master/mat64/lu.go#L224 need to be changed as well?

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.48%) when pulling d002f59 on solve into 2a243e3 on master.

@kortschak
Copy link
Copy Markdown
Member Author

That code is not called when the matrix is singular if using the Solve func. If the client is using the LU type directly, they can check IsSingular and deserve a panic if they don't.

@btracey
Copy link
Copy Markdown
Member

btracey commented Nov 3, 2014

LGTM

kortschak added a commit that referenced this pull request Nov 3, 2014
Return ErrSingular for singular matrix solves
@kortschak kortschak merged commit 6112e02 into master Nov 3, 2014
@kortschak kortschak deleted the solve branch November 3, 2014 05:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants