Move dotblas to multiarray #4969
Conversation
The current system works for MKL and OpenBLAS by default because the mkl_info and openblas_info classes in numpy/distutils/system_info do not define the macro 'NO_ATLAS_INFO=1' that currently signals the absence of CBLAS. This PR declares the presence of CBLAS directly by defining the 'HAVE_CBLAS' macro.
Importing _dotblas currently executes _dotblas.alterdot, which replaces the default descr->f->dot function with a cblas based version for float, double, complex float, and complex double data types. This PR changes the default descr->f->dot to use cblas whenever it is available. After this change, the alterdot and restoredot functions serve no purpose, so are changed to do nothing and deprecated. Note that those functions were already doing nothing when _dotblas was not available.
These are no longer needed. Also do C style cleanups.
|
Bento failed, works fine on my machine, but with waf-1.7.16, not 1.7.13. |
Move the dotblas_matrixproduct function from the _dotblas.c file to a new cblasfuncs.c file in multiarray and rename it cblas_matrixproduct. Modify it so that it can called directly from PyArray_MatrixProduct2 and do so. Fix up numeric.py and core/__init__.py to reflect these changes.
Move the dotblas_innerproduct function from the _dotblas.c file to a new cblasfuncs.c file in multiarray and rename it cblas_innerproduct. Modify it so that it can called directly from PyArray_InnerProduct and do so. Fix up numeric.py and the tests to reflect these changes.
|
This should be ready now. |
|
@pv Could you check the treatment of the ufunc override in the ndarray.dot method? I'm not clear on how that should work. Seems to me that calling |
Now that PyArray_MatrixProduct2 is blas enabled, call it directly.
Remove vdot from 8000 _dotblas and implement it in multiarray. Remove the files in core/blasdot as they are no longer needed. Fix tests and build to reflect the changes. The vdot function is now a bit faster in the common case as ravel is used instead of flatten. There is also no need to conjugate the files for clongdouble.
88f438b to
63cc74e
Compare
Also move docstrings into the versions in numpy/core/numeric.py as the functions are no longer in the defunct _dotblas module.
Not all tests from test_blasdot were relevant, as they checked the cblas implementations against the non-cblas implementations. Those two are no longer separate. Remove test_blasdot.py as there nothing is left in it.
|
This PR is getting rather large, so I'd like to get it in and make the '@' operator another PR. |
| # python threads or processes to one core | ||
| import os | ||
| envbak = os.environ.copy() | ||
| if 'openblas_main_free' not in os.environ: |
54f00fa to
a746e3a
Compare
I had to move convert_shape_to_string to common.h so that _dotblas.c can access it.
|
how do I get numpy to link against system blas now? And how do I check if I have dotblas? |
|
hm do we still need the |
|
The BLAS detection is done the same way it was before by using |
|
filed charris#3 which should make using system blas easier |
Allows building against system installed blas library that might be optimized.
BLD: check for CBLAS header in "unoptimized" blas
|
@juliantaylor Probably only for backwards compatibility. I resisted the temptation to fool with the thing, but we might want to clean things up at some point. |
Renamed not_aligned to dot_alignment_error. Conflicts: numpy/core/src/multiarray/cblasfuncs.c numpy/core/src/multiarray/common.h
Better "matrices not aligned" message
On OpenBSD with gcc-4.2 log1p(nan) raises an invalid value warning. This PR disables both 'divide' and 'invalid' for the log1p tests. Closes numpy#5017.
…ctions MAINT: Make numpy/lib/polynomial/polyval a bit faster.
BUG: fix percentage reporting when testing.assert_allclose fails.
TST: Silence some warning that turns up on OpenBSD.
|
needs rebase, is it ready after that? |
ENH: optimize STRING_compare by using memcmp
* old-move-dotblas: STY: Add spaces around '-'. BLD: check for CBLAS header in "unoptimized" blas ENH: include shapes in "matrices not aligned" msg BUG: Capitalize environmental variables in numpy/core/__init__.py. ENH: np.dot: better "matrices not aligned" message TST: Add vdot tests, move tests from test_blasdot to test_multiarray. DOC: Update docs to reflect deprecation of alterdot and restoredot. ENH: Move vdot to multiarray. MAINT: Refactor ndarray.dot method to call PyArray_MatrixProduct2. ENH: Move dotblas_innerproduct down into multiarray. MAINT: Update waf to 1.7.16 ENH: Move dotblas_matrixproduct down into multiarray. MAINT, STY: Remove use of alterdot, restoredot in _dotblas.c. ENH: When cblas is available use it in descr->f->dot. ENH: Add 'HAVE_CBLAS' macro for build purposes. Conflicts: numpy/core/src/multiarray/arraytypes.c.src Move the functionality of the numpy/core/dotblas module down into multiarray. Add 'vdot' to the functions in the multiarray module. This avoids the need to overwrite the descr->f->dot functions when CBLAS is available and paves the way to implement the new '@' operator.
|
Rebase didn't work :( So I made a new branch off master and merged it there. |
| return 1; | ||
| } | ||
| else if (i < 0) { | ||
| return -1; |
There was a problem hiding this comment.
how did this get in here
There was a problem hiding this comment.
Looks like it came from master, I merged the old branch into a new branch off master as the rebase route was not giving the proper result. The only problem with merging the old branch, IIRC, was an include of npy_common.h.
There was a problem hiding this comment.
If you have a better idea of how to do that, let me know.
There was a problem hiding this comment.
well it can't be merged like this due to the conflict, I'll have a look if I can rebase it
There was a problem hiding this comment.
You might need to old branch, which is no longer on github.
There was a problem hiding this comment.
If I omit the larsmans/dot-errmsg-no-dotblas merge the rebase becomes easy. I think that branch is a bit messed up.
There was a problem hiding this comment.
yes it contains an unnecessary commit that screws everything up, if you squash merge it should work too
There was a problem hiding this comment.
Perhaps I can cherry pick the relevant commits.
There was a problem hiding this comment.
meh, just reset your branch to mine, the authorship is still in the ocmmit log
There was a problem hiding this comment.
Just put your branch up, that's fine with me.
Preliminary PR moving dotblas_matrixmultiply down into multiarray. It is not quite complete as the
innerfuntion should also be done, along with the new__matmult__method for Python 3.5. However, this lets me test current functionality and provide an opportunity for review. If it seems reasonable to merge this, I can make new PRs to finish the job.