Adddlasq routines and tests #68
Conversation
|
This is not ready for submission, but pretty close to get the right idea. Rather than the loops at the bottom just checking for infinite loops (which I had for a while), I'd like to turn that into a test of the behavior. The dlasq2 tests having a tolerance of 1e-6 I think is because of single precision issues I couldn't figure out. Fortran wasn't giving the same answer depending if I generated the numbers randomly vs. actually giving it the number. There are a lot of ugly parts of the code. The gotos are highly nontrivial. At least for now I'd like to leave all the weird loop index stuff. If we discover more bugs down the line it's way easier to debug this way than if the code is idiomatic go. I also have all the debugging scripts. I assume we don't want them in the tree, but they'll be very useful if we find bugs in the future. The coverage of paths on this is decent. It seems like the unused paths come in for underflow. |
|
My reading of the matrices is an is the construction of triDi. However, this constructed matrix is not symmetric (let alone positive definite). Am I interpreting something wrong? Does "The tridiagonal is LU or, if you prefer, the symmetric tridiagonal to which it is similar." mean one needs to transform LU? |
|
I'm a little confused by this PR. The additions, I'll take a look at tomorrow, but I don't understand what's happening with Dorglq and Dorgqr in cgo. I think that we may want the debugging code available - I was actually thinking of adding a cgo/internal/fortran to add utilities to help with direct use of fortran calls that are made available via the CL Seb proposed. This would fit in the same space. BTW I edited your comment above so the * does not get interpretted as italic. That was very confusing. |
|
I am also confused by those changes. Will fix. I can add the debugging code |
|
Part of the problem was my reading of the documentation for U. However, that doesn't solve the problem. Opened Stack Overflow question where maybe someone will see it. http://stackoverflow.com/questions/33614240/lapack-dlasq2-matrix-input-format My debugging code is all strictly Fortran with some print statements used to translate between fortran and go inputs. Should I include that code as a subfolder of lapack/native, lapack/testlapack, or somewhere else? |
|
@kortschak I don't understand. If I do There is no difference shown. Is this a bug with the github interface, or do I not know how to use git properly? |
|
Neither. There is no difference in native/dorglq.go between those two branches. Though, the command can be more easily said as |
|
Oh, I see, it's in the c interface. Sorry. I'll check that, and I'll clean up the Fortran code to remove a lot of the unnecessary print statements and comments. Otherwise I think the Go code is good enough to go. Working up to the full SVD will allow us to have better tests of the full integrated system. |
|
Okay, cleaned up the Fortran code. PTAL I know there's still a bunch of ugly code there, but I'd prefer to leave it as is until we have integrated SVD tests if that's okay. |
internal/dlasqtest/commands.txt
Outdated
There was a problem hiding this comment.
Make this something that can be executed, or at least sourced - prefix this line with #.
|
It seems to me that |
|
Are the whitespace diffs between the fortran code in dlasqtest and OpenBLAS/lapack-netlib/SRC there intentionally or residually? Just curious. |
internal/dlasqtest/dlasq6.f
Outdated
There was a problem hiding this comment.
I'm not sure. None of the random test cases I generated ever reached dlasq6. My error rate was low in translation, but not non-zero (and sometimes in tricky ways). Part of me wants a trigger in dlasq6.go so when a case is found we can debug it.
|
I can't comment on testlapack/dlasq2-5 because of github limits presumably. So... dlasq2.go L26 - delete commented out interface dlasq2.go L638 - do these need to stay? dlasq3.go L2643 - use t.Error or show values dlasq3.go L2646-2697 - use idiomatic test failure phrasing dlasq4.go L24-54 - move these out to a fortran helper file. If you make a fortran64 type with a String method the code for string representation of float64 will read better. All - add copyright header. |
|
What is native/c.out? |
There was a problem hiding this comment.
Here dmin is actually an out only, so drop it from the input parameter list, also dmin1, dmin2, dn, dnm1 and dnm2.
There was a problem hiding this comment.
It's not that simple. You'll notice that the first return happens, and that in the LAPACK function none of the values are initialized. Well... since Fortran passes by reference their value is preserved through the call. It seems like they are actually outputs here (the tests don't fail if I add it), but I ran into these bugs in Dlasq4. I added a TODO.
There was a problem hiding this comment.
Yes it is. Not a fun bug to find. I fixed the first one, found a second bug along those lines, and then just decided not to believe the documentation and make the function behave like the Fortran one. I probably overreacted, but we can fix later.
|
OK finished first pass. @vladimir-ch Would you please take a look as well. There is a lot here and even a low review error rate (which I don't guarantee) is troubling. |
|
There is a binary file |
native/dlasq5.go
Outdated
There was a problem hiding this comment.
No, I meant outputs, but I improved the documentation.
|
I think that with some of the issues you've found here due to pass through, we should not allow unnamed returns in native. There are a few cases elsewhere that do. |
There was a problem hiding this comment.
I will be doing something wrong but I get a segfault at dlasq3.f:246.
There was a problem hiding this comment.
You're right.... how did that happen?
There was a problem hiding this comment.
Oh, it's because there isn't any data in testdlasq3 at the moment.
|
Finally had some time for the first time in two weeks to take a superficial look at this. I'm not sure if my comments are helpful and if I will be able to look into this deeper. |
|
Thanks for the commentary. Onward to routines we can write real tests for! |
No description provided.