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

Conversation

@btracey
Copy link
Member

@btracey btracey commented Aug 20, 2015

No description provided.

@btracey
Copy link
Member Author

btracey commented Aug 20, 2015

Not ready for submission.

Dlarts is necessary for Dgecon (Compute condition number from QR facotrization) and Dtrcon (compute condition of triangular matrix, which can be used from LU to compute the condition number).

In the normal case, this function mostly just calls Dtrsv. In the case where the matrix is badly conditioned, it does all sorts of crazy things. Any ideas on testing? For the badly conditioned cases in BLAS I created the ill conditioned cases by hand (for the Givens rotation cases). Here though, there are many possible code paths, so I can't construct a suffient set by hand. Additionally, it's meant to protect against bad scaling, so we can't compare the output to Dtrsv (because it shouldn't be the same), and I'm worried about testing that A * x = b, because presumably we would need also need to be careful with scaling. At the least, I'll need to do a second pass with visual inspection.

@kortschak
Copy link
Member

You should be able to use dvykov's go-fuzz to build a minimal corpus that tests all paths with some fiddling.

@jonlawlor
Copy link
Contributor

If you're going to try out fuzz, I think you'll first want to write byte encoders / decoders for the various blas types.

@kortschak
Copy link
Member

kortschak commented Aug 20, 2015 via email

@btracey
Copy link
Member Author

btracey commented Aug 21, 2015

We have one

One what? A go-fuzz example, or a comparison for triangular solve?

@kortschak
Copy link
Member

We have a binary encoder. This means that a fuzzer can be written with a corpus of encoded matrices from the original handwritten test cases. The other matrix types can be created from the decoded Dense.

@btracey
Copy link
Member Author

btracey commented Sep 4, 2015

Okay, I ran a fuzzer against it for a while so I think it's free of panics. Unfortunately, there is no cgo version of the algorithm so I cannot compare outputs.

PTAL

@kortschak
Copy link
Member

Pity Dlatrs doesn't exist in the cgo. I'll have a look some time over the weekend.

@kortschak
Copy link
Member

Change the PR title to reflect the function name?

@btracey btracey changed the title Start adding dlarts Add dlarts Sep 4, 2015
native/dlatrs.go Outdated
Copy link
Member

Choose a reason for hiding this comment

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

s/notrans/noTrans/g
s/nonunit/nonUnit/g

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@kortschak
Copy link
Member

LGTM with minor comments.

btracey added a commit that referenced this pull request Sep 5, 2015
@btracey btracey merged commit a76c841 into master Sep 5, 2015
@btracey btracey deleted the adddlarts branch September 5, 2015 19:33
@kortschak
Copy link
Member

kortschak commented Sep 5, 2015 via email

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