FS#19368 - [blas] bug in SVD routine srotm.f/drotm.f

Attached to Project: Arch Linux
Opened by Leonid Isaev (lisaev) - Wednesday, 05 May 2010, 21:00 GMT
Last edited by Jan de Groot (JGC) - Wednesday, 09 June 2010, 09:02 GMT
Task Type Bug Report
Category Upstream Bugs
Status Closed
Assigned To Ronald van Haren (pressh)
Architecture All
Severity High
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

Description:

This is just to let people know, because it's an upstream issue.

There seems to be a bug in BLAS, if compiled with gfortran -fcheck=all (gcc 4.5) or -fbounds-check (gcc < 4.5). The error is revealed, for instance, when one performs testing of lapack: make lapack_testing

SVD: Testing Singular Value Decomposition routines
./xeigtsts < svd.in > ssvd.out
At line 74 of file srotm.f
Fortran runtime error: Index '2' of dimension 1 of array 'sx' above upper bound of 1
make: *** [ssvd.out] Error 2

I also attach the output from gdb.

Additional info:
* package version(s)

blas 20070405-2

* config and/or log files etc.
gdb.out
   gdb.out (3.4 KiB)
This task depends upon

This task blocks these from closing
 FS#19745 - [blas] needs gcc-fortran as makedep 
Closed by  Jan de Groot (JGC)
Wednesday, 09 June 2010, 09:02 GMT
Reason for closing:  Implemented
Additional comments about closing:  lapack and blas build from one PKGBUILD now.
Comment by Ronald van Haren (pressh) - Thursday, 06 May 2010, 08:32 GMT
I take it the binary package in the repo is not affected?

Please also follow this up with upstream, there is a contact adress on their website.
Comment by Leonid Isaev (lisaev) - Thursday, 06 May 2010, 20:29 GMT
1. This is a bug in the source, so the binary package is affected. However, most likely this is a triviality:

real::sx(1)
...
if ((.false.).and.(sx(2)>0)) then
...
end if

So, potentially dangerous directives inside the if..endif will not be evaluated, but the compiler still computes the condition in (...), and execution may give segfault, because sx(2) is undefined. However, this is only a result of a quick glance at the file.

2. Done, please see the attached email.

PS: Also, I apologize for the inaccuracy in the title -- BLAS doesn't have SVD: s(d)rotm.f is an auxillary rotation subroutine. Please change if you feel necessary, because I can't edit the original post.
   lapack (5.2 KiB)
Comment by Leonid Isaev (lisaev) - Sunday, 09 May 2010, 05:24 GMT
OK, my complaint was rather uneducated.

The BLAS, which we package (www.netlib.org/blas) indeed has multiple bugs (as can be demonstrated by the attached trivial testcase). However, these are fixed in the version, shipped with lapack. So, I suggest to merge the two packages into one PKGBUILD (attached below).

PS: I also don't understand the relevance of blas-link.patch... is it to make only gfortran ... -llapack and not -lblas -llapack?
Comment by Ronald van Haren (pressh) - Monday, 10 May 2010, 09:30 GMT
the patch was to link dynamically against blas, suppose it is not needed if we use the one inside the lapack package.

It may be me, but I find it rather strange that they still have the blas package up on their homepage but in fact merged it with the lapack package (or I may have missed the note on their website). Either way, I'll switch the package over.

If I'm not mistaken one would need both lapack and blas for most usecases, so there is no need to split both packages from the lapack source (at least all official packages depend on both) ?

Comment by Leonid Isaev (lisaev) - Monday, 10 May 2010, 15:48 GMT
Well, lapack depends on blas, but nothing depends on lapack.

They don't include blas in the lapack package, but rather provide an option to build it via make blaslib. I just ran diff and saw that the invalid array declarations were corrected in lapack's blas source.

>It may be me, but I find it rather strange that they still have the blas package up on their homepage but in fact merged it with the >lapack package (or I may have missed the note on their website). Either way, I'll switch the package over.

Thanks, but can you wait for a little, while they get back to me over email, since I am going to ask them this question? I'll post here when it happens.

>the patch was to link dynamically against blas, suppose it is not needed if we use the one inside the lapack package.

My understanding (and this is also the authors' recommendation) is that it is better to use third-party BLAS, which is supposedly more optimized than the netlib's (or lapack's) one. While in practice it may not matter, I still think that the right way to link is -llapack -lmyblas, and not only -llapack.

This also means that if there is one package for both, people who use their own blas will end up with two blas libs...
Comment by Ronald van Haren (pressh) - Tuesday, 11 May 2010, 08:27 GMT
sure, just ping me with the preferred solution when you have all the information you need.
Comment by Leonid Isaev (lisaev) - Thursday, 20 May 2010, 01:06 GMT
Ronald, sorry for a delay.

I got the reply from lapack/blas devs. They acknowledge that there is a problem with netlib's blas, which was corrected in lapack's one. They also say that the blas at netlib will probably be upgraded soon, but I don't know when...

Regarding packaging, I think the decision should be based on your convenience (maintanence, etc.). In my opinion, people who really need these packages (on an Arch cluster, for instance) must compile their own libs [otherwise they will get what they deserve :)], while those who use Arch in a development environment (myself included) don't care much about performance and probably just need everything in one package for simplicity. That's my motivation for merging the two packages...

Loading...