blaspp with blas implementation other than MKL & how to contribute

[Vincent Picaud]

First, Happy new year to everybody.

I would like to use blaspp and lapackpp for my own work.

The first step is to check if I can compile blaspp with my setting: Linux + OpenBlas.

However, as far as I undertand, the current release mostly assumes a MKL blas implementation.

Actually if I try to compile blaspp with OpenBlas I got some compilation errors for blas subroutines involving complex number arguments.

Problem description, cblas_scasum example:

From cblas.h headers we see,

MKL         : float cblas_scasum(const MKL_INT n, const void *x, const MKL_INT incx);
OpenBlas : float cblas_scasum(OPENBLAS_CONST blasint n, OPENBLAS_CONST float  *x, OPENBLAS_CONST blasint incx);

When MKL is used, std::complex<>* is implicitly converted to void*, which is legal C++.
However for blas implementations like OpenBlas (and other like AtlasBlas...), the conversion to float* is  illegal, hence a compile time error.

One cure I routinely use is to systematically wraps std::complex arguments as follow: cblas_scasum(n,blas::cplx2ptr(x),incs);

where blas::cplx2ptr(x) is defined by:

  template <typename T>
  inline T* cplx2ptr(std::complex<T>* x) noexcept
  {
    return reinterpret_cast<T*>(x);
  }
  template <typename T>
  inline const T* cplx2ptr(const std::complex<T>* x) noexcept
  {
    return reinterpret_cast<const T*>(x);
  }

I have forked blaspp to fix that.

Now my questions are:

-> do you agree with that? (maybe I miss something)
-> are you open to pull request by potential contributors?

In case you are open to PR, I also have a pragmatic problem to format the C++ code according to your style.

-> are you using clang-format or another automatic formating tool? In that case is it possible to include the style format (.clang-format file for instance) in the release?


Best,
Vincent

-------------------------

nb: another minor fix to compile blaspp, AFAIK and according to the C++ standard, the va_start function used in blas_util.hh requires the <cstdarg> header.

[Mark Gates]

Hi Vincent,

Yes, I’m not surprised that it will take some work to make it portable to a variety of platforms. It would be helpful to have the actual error output from your compilation (the first few errors, not pages and pages of errors that C++ sometimes generates). Otherwise it is difficult to understand what issue you are encountering. I’m guessing this is just a problem in compiling the testers, with blaspp/test/cblas.hh.

From your code snippets below, it appears quite unfortunate that OpenBLAS changed cblas.h to be incompatible with the standard cblas.h.

I don’t use any auto formatting tool. There is a style guide at

https://bitbucket.org/icl/style/wiki/ICL_C_CPP_Coding_Style_Guide

It may be a little different; keep the code itself consistent.

  -mark

--
You received this message because you are subscribed to the Google Groups "SLATE User" group.
To unsubscribe from this group and stop receiving emails from it, send an email to slate-user+...@icl.utk.edu.
To post to this group, send email to slate...@icl.utk.edu.
To view this discussion on the web visit https://groups.google.com/a/icl.utk.edu/d/msgid/slate-user/07f2c3d0-9923-4742-a0e3-0f8c3497005a%40icl.utk.edu.

[Vincent Picaud]

Hi mark,

Thanks for the feedback.

Ok for C++ coding style.

My problem does not come from libtest, it compiles without problem.

To compile blaspp with generic blas implementation I have defined a "generic" make.inc.linux.generic as follow:

# Generic linux framework with GNU gcc/g++
#
# Create a symbolic link:
#     ln -s make.inc.linux.generic make.inc
#
# Assumes $CBLASDIR is set to where cblas.h exists, e.g., in ~/.profile
#     export CBLASDIR=/usr/include/x86_64-linux-gnu/
#
# Then
#     make

CXX       ?= g++

LDFLAGS   = -fPIC -fopenmp
CXXFLAGS  = -fPIC -fopenmp -MMD -std=c++11 -pedantic \
            -Wall -Wmissing-declarations \
            -Wno-unused-local-typedefs

CXXFLAGS += -I${CBLASDIR} \
            -DBLAS_COMPLEX_RETURN_ARGUMENT

# caveat: one must link against lapack due to test/lapack_tmp.hh used in all blaspp tests.
LIBS      = -llapack -lcblas -lblas

Trying to compile it, I got the following outputs:

g++ -fPIC -fopenmp -MMD -std=c++11 -pedantic -Wall -Wmissing-declarations -Wno-unused-local-typedefs -I -DBLAS_COMPLEX_RETURN_ARGUMENT -I../libtest -Iinclude -c -o test/test_symv.o test/test_symv.cc
In file included from test/test.hh:5:0,
                 from test/test_symv.cc:3:
include/blas_util.hh: In function ‘void blas::internal::throw_if(bool, const char*, const char*, const char*, ...)’:
include/blas_util.hh:357:9: error: ‘va_start’ was not declared in this scope
         va_start( va, format );
         ^~~~~~~~
include/blas_util.hh:357:9: note: suggested alternative: ‘__sqrt’
         va_start( va, format );
         ^~~~~~~~
         __sqrt
In file included from test/test_symv.cc:4:0:
test/cblas.hh: In function ‘float cblas_asum(int, const std::complex<float>*, int)’:
test/cblas.hh:212:37: error: cannot convert ‘const std::complex<float>*’ to ‘const float*’ for argument ‘2’ to ‘float cblas_scasum(blasint, const float*, blasint)’
     return cblas_scasum( n, x, incx );
                                     ^
test/cblas.hh: In function ‘double cblas_asum(int, const std::complex<double>*, int)’:
test/cblas.hh:219:37: error: cannot convert ‘const std::complex<double>*’ to ‘const double*’ for argument ‘2’ to ‘double cblas_dzasum(blasint, const double*, blasint)’
     return cblas_dzasum( n, x, incx );
                                     ^
test/cblas.hh: In function ‘void cblas_axpy(int, std::complex<float>, const std::complex<float>*, int, std::complex<float>*, int)’:
test/cblas.hh:248:46: error: cannot convert ‘std::complex<float>*’ to ‘const float*’ for argument ‘2’ to ‘void cblas_caxpy(blasint, const float*, const float*, blasint, float*, blasint)’
     cblas_caxpy( n, &alpha, x, incx, y, incy );
                                              ^
test/cblas.hh: In function ‘void cblas_axpy(int, std::complex<double>, const std::complex<double>*, int, std::complex<double>*, int)’:
test/cblas.hh:257:46: error: cannot convert ‘std::complex<double>*’ to ‘const double*’ for argument ‘2’ to ‘void cblas_zaxpy(blasint, const double*, const double*, blasint, double*, blasint)’
     cblas_zaxpy( n, &alpha, x, incx, y, incy );

... long list of compile-time errors.

Notes:
- the first error is easily fixed by including <cstdarg> header (my initial post)

- the origin of the other errors is explained in my initial post. However after investigations here is an important update:

Update: I have checked the last OpenBlas release. I just discovered that they have changed float*,double* to void* very recently (5 Nov 2017)
-> see https://github.com/xianyi/OpenBLAS/commit/66ac898f6441f0cb334f76d0c5603c37962bf368#diff-d23268747e861f1166acbfc56e8b2917
So, please ignore my remark concerning this problem, I apologize I was not aware of these very last OpenBlas commits.

Conclusion:
I will update to the very last release of OpenBlas and check again if I can compile blaspp without modification.
After that I will send you my observation here.
Learned lesson: IHMO blaspp is not compatible, without fixes, with OpenBlas for release < 5 Nov 2017.


-------------------

Another observation: in my version, trsm test failed. Unfortunately I can not compare with the one using MKL, hence I am not sure if the error is mine or if it is already present in your repo.

./test trsm
input: ./test trsm
                                                                                          BLAS++       BLAS++       BLAS++         Ref.         Ref.       
  type  layout    side    uplo    trans     diag       m       n      alpha   align        error     time (s)      Gflop/s     time (s)      Gflop/s  status
     d     col    left   lower  notrans  nonunit     100     100     3.1416       1   1.6935e+04       0.0024       0.4159       0.0004       2.4895  FAILED
     d     col    left   lower  notrans  nonunit     200     200     3.1416       1   9.1367e+19       0.0059       1.3656       0.0018       4.4703  FAILED
     d     col    left   lower  notrans  nonunit     300     300     3.1416       1   2.4850e+47       0.0038       7.0353       0.0054       4.9701  FAILED
     d     col    left   lower  notrans  nonunit     400     400     3.1416       1   1.6215e+56       0.0045      14.1378       0.0120       5.3194  FAILED
     d     col    left   lower  notrans  nonunit     500     500     3.1416       1   1.5502e+79       0.0077      16.1667       0.0258       4.8465  FAILED


Best,
Vincent

[Vincent Picaud]

UPDATE:
- with the latest OpenBlas, besides the missing <cstdarg> header, blasspp compilation is OK

[Mark Gates]

Excellent. Thanks for the report. Glad to see OpenBLAS updating to be compatible.

  -mark

[Vincent Picaud]

To be clear I just forgot to mention that with openblas, one must remove the "-DBLAS_COMPLEX_RETURN_ARGUMENT".

This is important, as otherwise the code compiles but you get wrong results for some function involving complex numbers, like cdotc()
-> this explains why I had some failure in my previous tests
-> now the situation is clear: compilation with OpenBlas + tests are OK
--> note: would be nice to have a compile-time check for BLAS_COMPLEX_RETURN_ARGUMENT, as the code compile without error, but some subroutines are not working properly .

I also checked other blas-implementations like: netlib-blas or atlas-blas, however there are compilation errors.
This would be not too hard to fix however it  is not my priority, as openblas and mkl are certainly the most used implementations.

Best,
Vincent