#158 CBLAS and BLAS mismatch for non-positive increments

Both
closed-fixed
5
2011-04-14
2010-09-09
No

The cblas interface for nrm2, asum, amax, sscal have behaviour which
does not match that of BLAS.

For example in snrm2 the function ignores the sign of incX.

float cblas_snrm2(const int N, const float *X, const int incX)
{
if (N > 0)
{
if (incX < 0) return(ATL_snrm2(N, X, -incX));
else return(ATL_snrm2(N, X, incX));
}
else return(0.0f);
}

Whereas if we look at reference blas snrm2.f looks like

IF (N.LT.1 .OR. INCX.LT.1) THEN
NORM = ZERO
ELSE IF (N.EQ.1) THEN
NORM = ABS(X(1))
ELSE
...

where if incX < 1, it will return 0. Interestingly, the f77
implementation ATL_srefnrm2.c has the correct behavior . The pertinent
line of code is

if( ( N < 1 ) || ( INCX < 1 ) ) return( ATL_sZERO );

Discussion

  • Francesco Callari

    Notice that this bug causes some GLS (Gnu Scientific Library) unit tests to fail when linked with ATLAS.

     
  • Sameer Agarwal

    Sameer Agarwal - 2010-09-15

    Hi Guys,
    I have attached a patch below which fixes this issue. I used the fortran blas on netlib as my reference.

    - Sameer

     
  • R. Clint Whaley

    R. Clint Whaley - 2010-10-19

    Guys,

    Thanks for the detailed report. However, the CBLAS are defined to handle negative increments differently than the fortran interface does, since C has pointers that can move around. Therefore, I think the handling of negative increments is correct in ATLAS C interface.

    The more problematic case is when incX=0. ATLAS should indeed match the F77 in such a case (I think the original F77 allowed incX=0, but it was later disallowed).

    I am moving this to the error queue for fixing before the next release. Can you scope your code and see if you are actually wanting the 0 answer when incX < 0 (which I don't intend to change)?

    Thanks,
    Clint

     
  • R. Clint Whaley

    R. Clint Whaley - 2010-10-19
    • assigned_to: nobody --> rwhaley
    • milestone: 111737 -->
    • labels: 497307 -->
    • status: open --> open-accepted
     
  • R. Clint Whaley

    R. Clint Whaley - 2010-10-19
    • milestone: --> Both
    • labels: --> Incorrect answer
     
  • R. Clint Whaley

    R. Clint Whaley - 2010-10-20

    OK, just scoped the CBLAS standard, and it *does not* handle negative increments differently than f77 (just misremembered). I have modified the basefiles to correspond with the f77 behavior, even though I think it is a bad behavior (and notice it wasn't added until 1993). So, this bug should be fixed when 3.9.27 is released.

     
  • Sameer Agarwal

    Sameer Agarwal - 2010-10-20

    Hi Clint,
    Thank you so much for taking care of this. I appreciate it.
    ATLAS rocks!
    Sameer

     
  • Sameer Agarwal

    Sameer Agarwal - 2010-10-20

    While we are at it, are you planning to backport this to the 3.8 series?

     
  • R. Clint Whaley

    R. Clint Whaley - 2010-10-21
    • status: open-accepted --> open-fixed
     
  • R. Clint Whaley

    R. Clint Whaley - 2010-10-21

    Can you please confirm that this problem is fixed in 3.9.27?

    You are right, this problem will need to be errataed for 3.8; I will certainly backport the fix; the question is will this be enough of a change to trigger a patch release. I'll have to look at the outstanding errors and figure it out.

    Are you guys still using the stable, and so need the backport (I'm guessing you just use your own patch)?

    Thanks,
    Clint

     
  • Francesco Callari

    Hi Clint,

    Yes, we are using the stable version, with sandwichmaker's patch applied (we belong to the same organization), so the problems is fixed as far we are concerned. However, as I pointed out previously, this bug causes some of the Gnu Scientific Library's tests to fail when it is configured to use ATLAS instead of its own blas implementation. Given the popularity of the GSL, it is likely that a backport would benefit more users of stable ATLAS.

     
  • R. Clint Whaley

    R. Clint Whaley - 2010-10-21

    OK, it looks like there are several issues in the current stable, so I would hope to issue a patch release eventually.

    Before I do that, I would like confirmation that the fix in 3.9.27 fixes the issues completely. Can someone use this new release unpatched on the tests that uncovered the problem originally, and let me know if the problems are all resolved?

    Thanks,
    Clint

     
  • R. Clint Whaley

    R. Clint Whaley - 2010-10-22

    Guys,

    I would have released a stable patch today if I had confirmation that my fix was correct.

    Thanks,
    Clint

     
  • R. Clint Whaley

    R. Clint Whaley - 2010-11-03

    Guys,

    Can either of you confirm that this is fixed? I would like to update the stable release the next time I have a few minutes for ATLAS.

    Thanks,
    Clint

     
  • R. Clint Whaley

    R. Clint Whaley - 2011-04-14

    No response, assuming fixed.

     
  • R. Clint Whaley

    R. Clint Whaley - 2011-04-14
    • status: open-fixed --> closed-fixed
     

Log in to post a comment.