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

Add wrapper for LAPACK _trexc methods (reordering of the Schur form) #12659

Merged
merged 1 commit into from
Sep 13, 2015

Conversation

mfasi
Copy link
Contributor

@mfasi mfasi commented Aug 17, 2015

I need this wrapper to implement some of the points of #5840 (I'm sure I need it for funm and unwm). I'd be glad to add a few tests, but I have no idea how to test such an operation. Suggestions are welcome.

@kshyatt kshyatt added the linear algebra Linear algebra label Aug 17, 2015
Ptr{$elty}, Ptr{BlasInt}, Ptr{$elty}, Ptr{BlasInt},
Ptr{BlasInt}, Ptr{BlasInt},
Ptr{$elty}, Ptr{BlasInt}),
&'V', &n,
Copy link
Member

Choose a reason for hiding this comment

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

Usually, we don't restrict the options supported by LAPACK so I'd prefer a version with a compq argument. I'd be fine with having a second method without the compq argument that calls the first with compq=='V'.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't know that, I simply used the trsen wrapper as a template. I can fix that one as well. Thank you for having a look at the commit, I'll correct what you suggested and add a few tests shortly.

Copy link
Member

Choose a reason for hiding this comment

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

Oh. I see. It would be great if you could change that in trsen as well and preferably in a separate commit.

@andreasnoack
Copy link
Member

Maybe try one or two reorderings and check that the eigenvalues are unaffected. The main point is to check that the wrapper is working. The two errors I've mentioned would probably have been found with such tests.

@andreasnoack
Copy link
Member

I think it is fine to add these wrappers, but we should probably not add new functionality to 0.4 so please be patient.

@mfasi mfasi force-pushed the lapack_trexc branch 5 times, most recently from 1a6063d to a0c4604 Compare August 30, 2015 08:57
@mfasi
Copy link
Contributor Author

mfasi commented Aug 30, 2015

@andreasnoack I added the tests you suggested, but some of the checks fail for some reason I cannot understand. For instance, I get a MethodError for Linux x86_64, but not for Linux i686. Moreover, the lapack.jl test file works on my machine (Linux x86_64). Is there something I am grossly overlooking?
[I attach the stack trace for reference.]

The command "/tmp/julia/bin/julia -e 'versioninfo()'" exited with 0.

127.62s$ export JULIA_CPU_CORES=4 && cd /tmp/julia/share/julia/test && /tmp/julia/bin/julia --check-bounds=yes runtests.jl all && /tmp/julia/bin/julia --check-bounds=yes runtests.jl pkg

    From worker 5:       * linalg/matmul         in  10.61 seconds

    From worker 5:       * linalg/schur          in  15.55 seconds

    From worker 5:       * linalg/special        in   4.40 seconds

    From worker 5:       * linalg/eigen          in  10.10 seconds

    From worker 5:       * linalg/bunchkaufman   in   5.24 seconds

    From worker 5:       * linalg/svd            in   8.56 seconds

    From worker 3:       * linalg/qr             in  57.18 seconds

    From worker 4:       * linalg/dense          in  58.58 seconds

    From worker 3:       * linalg/tridiag        in  14.93 seconds

    From worker 4:       * linalg/bidiag         in  14.80 seconds

    From worker 2:       * linalg/triangular     in 120.64 seconds

ERROR: LoadError: On worker 5:

LoadError("/tmp/julia/share/julia/test/linalg/lapack.jl",431,MethodError(trexc!,(1,2,

Float32[11.416281 -1.1872973 1.902823 -2.0817497

        0.0 6.4523597 -0.34416994 2.085003

        0.0 0.0 3.5656812 1.325061

        0.0 0.0 -0.1481058 3.5656812],

Float32[-0.5339776 -0.78375375 0.20146918 -0.24496534

        -0.3179281 0.021552414 -0.9365313 -0.14617346

        -0.75296825 0.5962653 0.27554345 -0.039776057

        -0.21639903 -0.17243204 -0.07997203 0.9576237])))

 in anonymous at no file:435

 in include_string at loading.jl:233

 in include_from_node1 at ./loading.jl:274

 in runtests at /tmp/julia/share/julia/test/testdefs.jl:179

 in anonymous at multi.jl:889

 in run_work_thunk at multi.jl:642

 in anonymous at task.jl:889

while loading /tmp/julia/share/julia/test/runtests.jl, in expression starting on line 13

    From worker 5:       * linalg/lapack        

The command "export JULIA_CPU_CORES=4 && cd /tmp/julia/share/julia/test && /tmp/julia/bin/julia --check-bounds=yes runtests.jl all && /tmp/julia/bin/julia --check-bounds=yes runtests.jl pkg" exited with 1.

@mfasi mfasi force-pushed the lapack_trexc branch 2 times, most recently from eab70ea to d044efc Compare September 2, 2015 17:20
@mfasi
Copy link
Contributor Author

mfasi commented Sep 2, 2015

Just to record that I solved the problem above. I'll just relax and wait for the end of the feature freeze.

@kshyatt
Copy link
Contributor

kshyatt commented Sep 13, 2015

@mfasi the feature freeze is done since we've branched and this looks possible to merge. Is it ready to go?

@mfasi
Copy link
Contributor Author

mfasi commented Sep 13, 2015

@kshyatt I think so, feel free to merge.

jakebolewski added a commit that referenced this pull request Sep 13, 2015
Add wrapper for LAPACK _trexc methods (reordering of the Schur form)
@jakebolewski jakebolewski merged commit ccd298e into JuliaLang:master Sep 13, 2015
@mfasi mfasi deleted the lapack_trexc branch September 13, 2015 16:53
@mfasi
Copy link
Contributor Author

mfasi commented Sep 13, 2015

@jakebolewski Great, thanks.

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

Successfully merging this pull request may close these issues.

4 participants