Skip to content

Python buffer objects can have negative strides. #782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 7, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 18 additions & 22 deletions docs/advanced/pycpp/numpy.rst
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,11 @@ specification.

struct buffer_info {
void *ptr;
size_t itemsize;
ssize_t itemsize;
std::string format;
int ndim;
std::vector<size_t> shape;
std::vector<size_t> strides;
ssize_t ndim;
std::vector<ssize_t> shape;
std::vector<ssize_t> strides;
};

To create a C++ function that can take a Python buffer object as an argument,
Expand Down Expand Up @@ -95,8 +95,8 @@ buffer objects (e.g. a NumPy matrix).
throw std::runtime_error("Incompatible buffer dimension!");

auto strides = Strides(
info.strides[rowMajor ? 0 : 1] / sizeof(Scalar),
info.strides[rowMajor ? 1 : 0] / sizeof(Scalar));
info.strides[rowMajor ? 0 : 1] / (py::ssize_t)sizeof(Scalar),
info.strides[rowMajor ? 1 : 0] / (py::ssize_t)sizeof(Scalar));

auto map = Eigen::Map<Matrix, 0, Strides>(
static_cat<Scalar *>(info.ptr), info.shape[0], info.shape[1], strides);
Expand All @@ -111,18 +111,14 @@ as follows:

.def_buffer([](Matrix &m) -> py::buffer_info {
return py::buffer_info(
m.data(), /* Pointer to buffer */
sizeof(Scalar), /* Size of one scalar */
/* Python struct-style format descriptor */
py::format_descriptor<Scalar>::format(),
/* Number of dimensions */
2,
/* Buffer dimensions */
{ (size_t) m.rows(),
(size_t) m.cols() },
/* Strides (in bytes) for each index */
m.data(), /* Pointer to buffer */
sizeof(Scalar), /* Size of one scalar */
py::format_descriptor<Scalar>::format(), /* Python struct-style format descriptor */
2, /* Number of dimensions */
{ m.rows(), m.cols() }, /* Buffer dimensions */
{ sizeof(Scalar) * (rowMajor ? m.cols() : 1),
sizeof(Scalar) * (rowMajor ? 1 : m.rows()) }
/* Strides (in bytes) for each index */
);
})

Expand Down Expand Up @@ -322,17 +318,17 @@ where ``N`` gives the required dimensionality of the array:
m.def("sum_3d", [](py::array_t<double> x) {
auto r = x.unchecked<3>(); // x must have ndim = 3; can be non-writeable
double sum = 0;
for (size_t i = 0; i < r.shape(0); i++)
for (size_t j = 0; j < r.shape(1); j++)
for (size_t k = 0; k < r.shape(2); k++)
for (ssize_t i = 0; i < r.shape(0); i++)
for (ssize_t j = 0; j < r.shape(1); j++)
for (ssize_t k = 0; k < r.shape(2); k++)
sum += r(i, j, k);
return sum;
});
m.def("increment_3d", [](py::array_t<double> x) {
auto r = x.mutable_unchecked<3>(); // Will throw if ndim != 3 or flags.writeable is false
for (size_t i = 0; i < r.shape(0); i++)
for (size_t j = 0; j < r.shape(1); j++)
for (size_t k = 0; k < r.shape(2); k++)
for (ssize_t i = 0; i < r.shape(0); i++)
for (ssize_t j = 0; j < r.shape(1); j++)
for (ssize_t k = 0; k < r.shape(2); k++)
r(i, j, k) += 1.0;
}, py::arg().noconvert());

Expand Down
30 changes: 15 additions & 15 deletions include/pybind11/buffer_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,31 +15,31 @@ NAMESPACE_BEGIN(pybind11)

/// Information record describing a Python buffer object
struct buffer_info {
void *ptr = nullptr; // Pointer to the underlying storage
size_t itemsize = 0; // Size of individual items in bytes
size_t size = 0; // Total number of entries
std::string format; // For homogeneous buffers, this should be set to format_descriptor<T>::format()
size_t ndim = 0; // Number of dimensions
std::vector<size_t> shape; // Shape of the tensor (1 entry per dimension)
std::vector<size_t> strides; // Number of entries between adjacent entries (for each per dimension)
void *ptr = nullptr; // Pointer to the underlying storage
ssize_t itemsize = 0; // Size of individual items in bytes
ssize_t size = 0; // Total number of entries
std::string format; // For homogeneous buffers, this should be set to format_descriptor<T>::format()
ssize_t ndim = 0; // Number of dimensions
std::vector<ssize_t> shape; // Shape of the tensor (1 entry per dimension)
std::vector<ssize_t> strides; // Number of entries between adjacent entries (for each per dimension)

buffer_info() { }

buffer_info(void *ptr, size_t itemsize, const std::string &format, size_t ndim,
detail::any_container<size_t> shape_in, detail::any_container<size_t> strides_in)
buffer_info(void *ptr, ssize_t itemsize, const std::string &format, ssize_t ndim,
detail::any_container<ssize_t> shape_in, detail::any_container<ssize_t> strides_in)
: ptr(ptr), itemsize(itemsize), size(1), format(format), ndim(ndim),
shape(std::move(shape_in)), strides(std::move(strides_in)) {
if (ndim != shape.size() || ndim != strides.size())
if (ndim != (ssize_t) shape.size() || ndim != (ssize_t) strides.size())
pybind11_fail("buffer_info: ndim doesn't match shape and/or strides length");
for (size_t i = 0; i < ndim; ++i)
for (size_t i = 0; i < (size_t) ndim; ++i)
size *= shape[i];
}

buffer_info(void *ptr, size_t itemsize, const std::string &format, size_t size)
buffer_info(void *ptr, ssize_t itemsize, const std::string &format, ssize_t size)
: buffer_info(ptr, itemsize, format, 1, {size}, {itemsize}) { }

explicit buffer_info(Py_buffer *view, bool ownview = true)
: buffer_info(view->buf, (size_t) view->itemsize, view->format, (size_t) view->ndim,
: buffer_info(view->buf, view->itemsize, view->format, view->ndim,
{view->shape, view->shape + view->ndim}, {view->strides, view->strides + view->ndim}) {
this->view = view;
this->ownview = ownview;
Expand Down Expand Up @@ -78,13 +78,13 @@ NAMESPACE_BEGIN(detail)

template <typename T, typename SFINAE = void> struct compare_buffer_info {
static bool compare(const buffer_info& b) {
return b.format == format_descriptor<T>::format() && b.itemsize == sizeof(T);
return b.format == format_descriptor<T>::format() && b.itemsize == (ssize_t) sizeof(T);
}
};

template <typename T> struct compare_buffer_info<T, detail::enable_if_t<std::is_integral<T>::value>> {
static bool compare(const buffer_info& b) {
return b.itemsize == sizeof(T) && (b.format == format_descriptor<T>::value ||
return (size_t) b.itemsize == sizeof(T) && (b.format == format_descriptor<T>::value ||
((sizeof(T) == sizeof(long)) && b.format == (std::is_unsigned<T>::value ? "L" : "l")) ||
((sizeof(T) == sizeof(size_t)) && b.format == (std::is_unsigned<T>::value ? "N" : "n")));
}
Expand Down
8 changes: 4 additions & 4 deletions include/pybind11/class_support.h
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ inline void enable_dynamic_attributes(PyHeapTypeObject *heap_type) {
#endif
type->tp_flags |= Py_TPFLAGS_HAVE_GC;
type->tp_dictoffset = type->tp_basicsize; // place dict at the end
type->tp_basicsize += sizeof(PyObject *); // and allocate enough space for it
type->tp_basicsize += (ssize_t)sizeof(PyObject *); // and allocate enough space for it
type->tp_traverse = pybind11_traverse;
type->tp_clear = pybind11_clear;

Expand All @@ -459,16 +459,16 @@ extern "C" inline int pybind11_getbuffer(PyObject *obj, Py_buffer *view, int fla
view->ndim = 1;
view->internal = info;
view->buf = info->ptr;
view->itemsize = (ssize_t) info->itemsize;
view->itemsize = info->itemsize;
view->len = view->itemsize;
for (auto s : info->shape)
view->len *= s;
if ((flags & PyBUF_FORMAT) == PyBUF_FORMAT)
view->format = const_cast<char *>(info->format.c_str());
if ((flags & PyBUF_STRIDES) == PyBUF_STRIDES) {
view->ndim = (int) info->ndim;
view->strides = (ssize_t *) &info->strides[0];
view->shape = (ssize_t *) &info->shape[0];
view->strides = &info->strides[0];
view->shape = &info->shape[0];
}
Py_INCREF(view->obj);
return 0;
Expand Down
54 changes: 37 additions & 17 deletions include/pybind11/eigen.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,22 @@ template <typename T> using is_eigen_other = all_of<
template <bool EigenRowMajor> struct EigenConformable {
bool conformable = false;
EigenIndex rows = 0, cols = 0;
EigenDStride stride{0, 0};
EigenDStride stride{0, 0}; // Only valid if negativestrides is false!
bool negativestrides = false; // If true, do not use stride!

EigenConformable(bool fits = false) : conformable{fits} {}
// Matrix type:
EigenConformable(EigenIndex r, EigenIndex c,
EigenIndex rstride, EigenIndex cstride) :
conformable{true}, rows{r}, cols{c},
stride(EigenRowMajor ? rstride : cstride /* outer stride */,
EigenRowMajor ? cstride : rstride /* inner stride */)
{}
conformable{true}, rows{r}, cols{c} {
// TODO: when Eigen bug #747 is fixed, remove the tests for non-negativity. http://eigen.tuxfamily.org/bz/show_bug.cgi?id=747
if (rstride < 0 || cstride < 0) {
negativestrides = true;
} else {
stride = {EigenRowMajor ? rstride : cstride /* outer stride */,
EigenRowMajor ? cstride : rstride /* inner stride */ };
}
}
// Vector type:
EigenConformable(EigenIndex r, EigenIndex c, EigenIndex stride)
: EigenConformable(r, c, r == 1 ? c*stride : stride, c == 1 ? r : r*stride) {}
Expand All @@ -86,6 +92,7 @@ template <bool EigenRowMajor> struct EigenConformable {
// To have compatible strides, we need (on both dimensions) one of fully dynamic strides,
// matching strides, or a dimension size of 1 (in which case the stride value is irrelevant)
return
!negativestrides &&
(props::inner_stride == Eigen::Dynamic || props::inner_stride == stride.inner() ||
(EigenRowMajor ? cols : rows) == 1) &&
(props::outer_stride == Eigen::Dynamic || props::outer_stride == stride.outer() ||
Expand Down Expand Up @@ -138,8 +145,8 @@ template <typename Type_> struct EigenProps {
EigenIndex
np_rows = a.shape(0),
np_cols = a.shape(1),
np_rstride = a.strides(0) / sizeof(Scalar),
np_cstride = a.strides(1) / sizeof(Scalar);
np_rstride = a.strides(0) / static_cast<ssize_t>(sizeof(Scalar)),
np_cstride = a.strides(1) / static_cast<ssize_t>(sizeof(Scalar));
if ((fixed_rows && np_rows != rows) || (fixed_cols && np_cols != cols))
return false;

Expand All @@ -149,7 +156,7 @@ template <typename Type_> struct EigenProps {
// Otherwise we're storing an n-vector. Only one of the strides will be used, but whichever
// is used, we want the (single) numpy stride value.
const EigenIndex n = a.shape(0),
stride = a.strides(0) / sizeof(Scalar);
stride = a.strides(0) / static_cast<ssize_t>(sizeof(Scalar));

if (vector) { // Eigen type is a compile-time vector
if (fixed && size != n)
Expand Down Expand Up @@ -200,7 +207,7 @@ template <typename Type_> struct EigenProps {
// Casts an Eigen type to numpy array. If given a base, the numpy array references the src data,
// otherwise it'll make a copy. writeable lets you turn off the writeable flag for the array.
template <typename props> handle eigen_array_cast(typename props::Type const &src, handle base = handle(), bool writeable = true) {
constexpr size_t elem_size = sizeof(typename props::Scalar);
constexpr ssize_t elem_size = sizeof(typename props::Scalar);
array a;
if (props::vector)
a = array({ src.size() }, { elem_size * src.innerStride() }, src.data(), base);
Expand Down Expand Up @@ -242,8 +249,14 @@ struct type_caster<Type, enable_if_t<is_eigen_dense_plain<Type>::value>> {
using Scalar = typename Type::Scalar;
using props = EigenProps<Type>;

bool load(handle src, bool) {
auto buf = array_t<Scalar>::ensure(src);
bool load(handle src, bool convert) {
// If we're in no-convert mode, only load if given an array of the correct type
if (!convert && !isinstance<array_t<Scalar>>(src))
return false;

// Coerce into an array, but don't do type conversion yet; the copy below handles it.
auto buf = array::ensure(src);

if (!buf)
return false;

Expand All @@ -252,10 +265,17 @@ struct type_caster<Type, enable_if_t<is_eigen_dense_plain<Type>::value>> {
return false;

auto fits = props::conformable(buf);
if (!fits)
return false; // Non-comformable vector/matrix types
// Allocate the new type, then build a numpy reference into it
value = Type(fits.rows, fits.cols);
auto ref = reinterpret_steal<array>(eigen_ref_array<props>(value));
if (dims == 1) ref = ref.squeeze();

int result = detail::npy_api::get().PyArray_CopyInto_(ref.ptr(), buf.ptr());

value = Eigen::Map<const Type, 0, EigenDStride>(buf.data(), fits.rows, fits.cols, fits.stride);
if (result < 0) { // Copy failed!
PyErr_Clear();
return false;
}

return true;
}
Expand Down Expand Up @@ -561,9 +581,9 @@ struct type_caster<Type, enable_if_t<is_eigen_sparse<Type>::value>> {
object matrix_type = module::import("scipy.sparse").attr(
rowMajor ? "csr_matrix" : "csc_matrix");

array data((size_t) src.nonZeros(), src.valuePtr());
array outerIndices((size_t) (rowMajor ? src.rows() : src.cols()) + 1, src.outerIndexPtr());
array innerIndices((size_t) src.nonZeros(), src.innerIndexPtr());
array data(src.nonZeros(), src.valuePtr());
array outerIndices((rowMajor ? src.rows() : src.cols()) + 1, src.outerIndexPtr());
array innerIndices(src.nonZeros(), src.innerIndexPtr());

return matrix_type(
std::make_tuple(data, innerIndices, outerIndices),
Expand Down
Loading