Skip to content

trmv! gives warning for moderately large matrices #1748

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

Closed
dlfivefifty opened this issue Sep 3, 2018 · 2 comments · Fixed by #1943
Closed

trmv! gives warning for moderately large matrices #1748

dlfivefifty opened this issue Sep 3, 2018 · 2 comments · Fixed by #1943
Milestone

Comments

@dlfivefifty
Copy link

julia> using LinearAlgebra

julia> BLAS.vendor()
:openblas64

julia> n = 10000; A = randn(n,n); b = randn(n); BLAS.trmv!('U', 'N', 'N', A, b);

WARNING unrolling of the trmv_U loop may give wrong results

The offending line seems to be https://github.com/xianyi/OpenBLAS/blob/e11126b26ada8d97b4a522e461ca92311653bfc6/driver/level2/trmv_U.c#L66

See discussion https://discourse.julialang.org/t/why-lapack-trtrs-not-blas-trsv/14239/3

@martin-frbg
Copy link
Collaborator

martin-frbg commented Sep 3, 2018

This is related to #1332. Is it the same hardware as in #1698 ? I did not expect that loop to be entered on "common" hardware after the hack, but clearly your definition of "moderately large" is different from mine. Probably best to fully disable the suspicious code block until the actual issue is found and fixed. (Too bad that these issues with original codes from GotoBLAS come up only now.)

@martin-frbg
Copy link
Collaborator

Actually BLAS-Tester results (both with the smallish default problem sizes and larger ones like ./xdl2blastst -R trmv -U 1 U -n 15000) suggest that the warning is unnecessary as the results appear to match the reference values even with unrolling of the gemv loop (as long as trmv is not allowed to be parallelized again - this conclusion from #1332 remains valid).

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

Successfully merging a pull request may close this issue.

2 participants