(double*) instead of (void*) for complex numbers in CBLAS

[claudius-hubig]

(copy of Debian bug 877883 )

Dear Maintainer,

OpenBLAS in version 0.2.19-3 currently installed on Debian Stretch as well as the version 0.2.20+ds-4 in Testing declares the cblas_*() as taking double* instead of void* for complex values. As an example, it declares cblas_zdscal as

void cblas_zdscal(OPENBLAS_CONST blasint N, OPENBLAS_CONST double alpha, double *X, OPENBLAS_CONST blasint incX);

This function takes an array of complex numbers X and scales them by the real value alpha.

In contrast to this, all other packages in Debian as well as the Intel MKL define cblas_zdscal as taking a void* for X, i.e.:

void cblas_zdscal(const int N, const double alpha, void *X, const int incX);

See e.g. Debian Codesearch or the MKL reference or the Netlib cblas.h.

The same difference applies to the cblas_zscal() function and all others:

$ dpkg -S /usr/include/cblas.h; dpkg -S /usr/include/openblas/cblas.h; grep zscal /usr/include/openblas/cblas.h /usr/include/cblas.h
libblas-dev: /usr/include/cblas.h
libopenblas-dev: /usr/include/openblas/cblas.h
/usr/include/openblas/cblas.h:void cblas_zscal(OPENBLAS_CONST blasint N, OPENBLAS_CONST double *alpha, double *X, OPENBLAS_CONST blasint incX);
/usr/include/cblas.h:void cblas_zscal(const int N, const void *alpha, void *X, const int incX);

I have only noticed the issue now as cblas.h has become part of the alternatives system and is currently directing to the OpenBLAS version in testing. In Stretch, if also libblas-dev is installed, its cblas.h is used and the difference hence hidden away.

I suspect that this problem is not more widespread because people tend to cast their arguments to (void*) explicitly, but the conversion std::complex<double>* => void* => double* is probably actually undefined…

There was also some discussion in 2013, but it seemed to go nowhere.

I would be very thankful if there was some way to avoid the explicit casts to (void*) in calls specifically for OpenBLAS.

[martin-frbg]

These are probably "historic" differences, already present in K.Goto's GotoBLAS from about 10 years ago.

[svillemot]

ATLAS and GSL CBLAS also uses void * for complex array.

It is clearly a problem if OpenBLAS does not provide the same API as the other CBLAS implementations.

[brada4]

Netlib has this:
void cblas_zdscal(const int N, const double alpha, void *X, const int incX);
Everybody else is wrong and deviant from reference implementation?

[svillemot]

@brada4 No, everybody except OpenBLAS has void *

[brada4]

alpha deviates too, does it cause anything more than simple compile/cast warning?
If using netlib cblas.h helps against warning in principle OpenBLAS provides identical API and can use that 'reference' header instead.

[svillemot]

@brada4 This is precisely what we used to do in Debian. But I recently changed it to use the implementation-specific version of the cblas.h header, hence this bug report. In the worst case, I can still roll back that change.

[brada4]

Your change is correct. All that appears to be cblas can use reference header.
I can make PR but you can beat me in speed this weekend.

[claudius-hubig]

alpha deviates too, does it cause anything more than simple compile/cast warning?

For zscal, It’s not just a warning but the same full-blown error as with the array case. Casting anything to void* is valid and, once I casted to void*, it can be cast to anything else (though not necessarily well-defined), but without a manual cast to void*, the compiler does not consider the Open(C)BLAS-function valid, while all other CBLAS implementations work just fine.

[brada4]

@martin-frbg what do you think about making openblas.h that includes netlib cblas.h on top of own special macros/functions, but otherwise stop breaking peoples' expectations?

[martin-frbg]

Not sure where a special openblas.h would come in, at least at the moment I am more inclined to change the problematic declarations in cblas.h and leave everything else in the headers as it currently is.

[brada4]

maybe rectify out specific functions, then append reference header?

[martin-frbg]

@claudius-hubig @sebastien-villemot I wonder if one of you could review (or at least compile-test) my PR #1347 before I let it loose onto the world ? Given how widespread the changes resulting from the requested change of the header were, I'd hate to have to self-approve it.

[claudius-hubig]

@martin-frbg I can only comment on the changes in cblas.h, which look all good to me. However, when trying to compile my code, it appears that there is still a remaining inconsistency in the use of openblas_complex_double, for example in the cblas_zdotu_sub function. In netlib, this function takes as last parameter another void* whereas here it takes a openblas_complex_double, which seems to be defined in as a custom struct.

At least in cblas.h, only the four functions cblas_cdotu_sub, cblas_cdotc_sub, cblas_zdotu_sub and cblas_zdotc_sub seem to use this custom struct. If I change their definition to also use void* for their last parameter, my code compiles, links and runs successfully (even without recompiling the rest of OpenBLAS).

I can’t really comment on the changes in the other files, as I don’t understand them, however, they seem to be straightforward casts to fit with the lower levels of OpenBLAS?

In any case, thank you for the effort you put into maintaining this huge project!

[martin-frbg]

Good catch on the overlooked parameter, I will update the PR accordingly. Being late to the project has its advantages, but the potential for causing huge code breakage is definitely not one of them. :-)