xt::linalg::dot doesn't compile when used with xt::pytensor objects

[pdumon]

When I use xt::linalg::dot on xt::pytensor objects (from xtensor-python) I get two compilation errors for lines 750 and 841 in xlinalg.h:

xtensor-blas/xlinalg.hpp(841): error C2398: Element '2': conversion from 'const _Ty' to 'T' requires a narrowing conversion with [ _Ty=npy_intp ] and [ T=size_t ]

This is where a resize is done in xlinalg.hpp: result.resize({t.shape()[0], o.shape()[1]});

I could temporarily solve it by explicit casting to size_t on those lines 841 and 750, but not sure what the proper solution would be?

This is with xtensor-blas 0.17.1, on Windows with msvc 2017 v15.9: cl: Microsoft (R) C/C++ Optimizing Compiler Version 19.16.27034 for x64

[JohanMabille]

I think that adding these static_cast is the proper solution. linalg::dot should work with any kind of expressions, and you cannot guarantee that the type used for the size and the shape is always unsigned. Would you like to open a PR?

[pdumon]

From here https://github.com/scikit-learn/scikit-learn/wiki/C-integer-types:-the-missing-manual it seems that though generally npy_intp should have the same size as size_t (64-bit on a regular 64-bit unix or windows system) there's no guarantee for that. So there might be a small risk (on more exotic platforms?) to actually narrow the type?

But it seems shape in numpy is always signed so it would be limited to half the size of an (unsigned) xtensor shape anyway, so casting to size_t should be OK, correct?

[pdumon]

I guess my question would also be: where is the correct place for this cast: here in xtensor-blas, or somewhere in xtensor-python (so that doing xt::pyarray.shape() has the same return type as xt::xarray.shape()

[pdumon]

(By the way: kudos to QuantStack, it's a really nice library)

[JohanMabille]

But it seems shape in numpy is always signed so it would be limited to half the size of an (unsigned) xtensor shape anyway, so casting to size_t should be OK, correct?

Indeed, I don't think there is any issue in static casting to size_t on most platforms. For the exotic ones, let's see if someone opens issue ;)

I definitely think that the cast should go in this library, the shape of pyarray must hold (and return) values of the same type as those of numpy array's shape since pyarray allows to manipulate a numpy array in place.

[pdumon]

Ok! Thanks. Will create a PR. And then try to solve blas linking problems (using conda and cmake, installed openblas but cmake doesn't find blas for the moment) :-)

On Mon, Mar 9, 2020, 07:20 Johan Mabille notifications@github.com wrote:

But it seems shape in numpy is always signed so it would be limited to half the size of an (unsigned) xtensor shape anyway, so casting to size_t should be OK, correct?

Indeed, I don't think there is any issue in static casting to size_t on most platforms. For the exotic ones, let's see if someone opens issue ;)

I definitely think that the cast should go in this library, the shape of pyarray must hold (and return) values of the same type as those of numpy array's shape since pyarray allows to manipulate a numpy array in place.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/xtensor-stack/xtensor-blas/issues/155?email_source=notifications&email_token=ALSEKY6BCCTWGQ4G5W6LBSDRGT3LDA5CNFSM4LD7T5Z2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOHMQMA#issuecomment-596559920, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALSEKY2S2XORFZ4WENUKK3TRGT3LDANCNFSM4LD7T5ZQ .

[JohanMabille]

I had this issue too, for an unknown reason conda installs openblas 0.3.6 while it used to install 0.3.7. Forcing the version to 0.3.7 fixed the issue in travis-ci.

[pdumon]

Created PR but doesn't seem to be automatically linked to the issue