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

Add Dpocon and tests #45

Merged
merged 3 commits into from
Sep 10, 2015
Merged

Add Dpocon and tests #45

merged 3 commits into from
Sep 10, 2015

Conversation

btracey
Copy link
Member

@btracey btracey commented Sep 10, 2015

No description provided.

@btracey
Copy link
Member Author

btracey commented Sep 10, 2015

This one at least matches Dgecon. No deterministic at the start because the matrix multiplied by itself is singular.

if len(iwork) < n {
panic(badWork)
}
clapack.Dpocon(uplo, n, a, lda, anorm, rcond)
Copy link
Member

Choose a reason for hiding this comment

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

Missing return.

@kortschak
Copy link
Member

LAPACKE seems to be pretty unhealthy.

@btracey
Copy link
Member Author

btracey commented Sep 10, 2015

As a library or as something that works with Travis?

@kortschak
Copy link
Member

OpenMathLib/OpenBLAS#639

@btracey
Copy link
Member Author

btracey commented Sep 10, 2015

Ah. Not good. This kind of thing makes me think we're the only ones who have tested a bunch of this interface ...

I'd like to just say "disable the cgo test", but I'm more worried about that here since we actually plan on calling this function directly. The cgo tests also have the LQ factorizaton test commented out because of a bug. That's a significant chuck of Solve that won't work with a c-based lapack. I guess the answer is to make sure that the mat64 tests are robust, so the issues are visible on that level? I don't want to abandon clapack altogether because it's a useful check, but at the same time it's frustrating how many bugs we're finding (in such a widely used interface at that!)

@kortschak
Copy link
Member

Indeed. I'm going to spend a little time on seeing if I can figure out why it's broken.

I think we should disable the cgo test in the interim.

@btracey
Copy link
Member Author

btracey commented Sep 10, 2015

Test disabled.

@kortschak
Copy link
Member

Fix found. You will not believe it.

@btracey
Copy link
Member Author

btracey commented Sep 10, 2015

Oh no!

@kortschak
Copy link
Member

LGTM

@kortschak
Copy link
Member

The cgo failures on my machine:

--- FAIL: TestDpocon (0.01s)
    dpocon.go:126: Cond mismatch. Want 0.056005662446310195, got 0.021282043474993715.
    dpocon.go:126: Cond mismatch. Want 0.09149436558852704, got 0.09181637098950025.

Do you want to send a PR to OpenBLAS for the fix you have?

@kortschak
Copy link
Member

BTW What is the bug that's blocking Dormlq?

@btracey
Copy link
Member Author

btracey commented Sep 10, 2015

I'm not sure what you mean by "The fix I have". The Go version just works, and I haven't been able to test this with cgo because of the openblas failures.

LQ problem is OpenMathLib/OpenBLAS#615

@kortschak
Copy link
Member

Oh, sorry, conflating different threads (Dtrcon).

btracey added a commit that referenced this pull request Sep 10, 2015
@btracey btracey merged commit 649c46e into master Sep 10, 2015
@btracey btracey deleted the adddpocon branch September 10, 2015 13:59
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.

2 participants