xtensor-stack / xtensor-python

Python bindings for xtensor
BSD 3-Clause "New" or "Revised" License
347 stars 58 forks source link

support for noconvert #73

Closed nbecker closed 7 years ago

nbecker commented 7 years ago

It seems noconvert is not supported? I tried adding noconvert to my (now classic) summer.cc:

#include <numpy/arrayobject.h>
#include "pybind11/numpy.h"     // complex
#include "pybind11/pybind11.h"
#include "pybind11/operators.h"
#include "numpy/arrayobject.h"
//#include "ndarray/pybind11.h"
#include "xtensor/xtensor.hpp"
#include "xtensor/xcontainer.hpp"
#include "xtensor-python/pyarray.hpp"

#include <complex>

namespace py = pybind11;

PYBIND11_PLUGIN (summer) {
  if (_import_array() < 0) {
    PyErr_SetString(PyExc_ImportError, "numpy.core.multiarray failed to import");
    return nullptr;
  }

  py::module m("summer", "pybind11 example plugin");

  // m.def("sum", [](xt::pyarray<double> const& x) {
  //     xt::xtensor<double, 1> tmp(x);
  //     double sum = 0;
  //     for (auto e : tmp)
  //       sum += e;
  //     return sum;
  //   });

  m.def("sum", [](xt::pyarray<std::complex<double>> const& x) {
      std::complex<double> sum = 0;
      for (auto e : x)
        sum += e;
      return sum;
    },
    py::arg("inp").noconvert()
    );

  m.def("sum", [](xt::pyarray<double> const& x) {
      double sum = 0;
      for (auto e : x)
        sum += e;
      return sum;
    },
    py::arg("inp").noconvert()
    );

  return m.ptr();
}

It seems to have no effect. If I reverse the order of the defs (double is 1st), then when passing a complex array I get the numpy warning of discarding imag part. If I use the order above, the time for processing float array is much more than for complex array, suggesting there is conversion.

SylvainCorlay commented 7 years ago

Thanks for the pointers. We should probably support this.

nbecker commented 7 years ago

I don't really know what I'm doing here, but this seems to work

diff -r 53dcd0192dc2 include/xtensor-python/pytensor.hpp
--- a/include/xtensor-python/pytensor.hpp   Sat Apr 08 15:46:37 2017 +0200
+++ b/include/xtensor-python/pytensor.hpp   Wed Apr 12 08:04:52 2017 -0400
@@ -43,9 +43,16 @@
         {
             using type = xt::pytensor<T, N>;

-            bool load(handle src, bool)
+            bool load(handle src, bool convert)
             {
-                value = type::ensure(src);
+              if (!convert) {
+                if (!PyArray_Check(src.ptr()))
+                  return false;
+                int type_num = xt::detail::numpy_traits<T>::type_num;
+                if (PyArray_TYPE(src.ptr()) != type_num)
+                  return false;
+              }
+              value = type::ensure(src);
                 return static_cast<bool>(value);
             }

@@ -56,6 +63,26 @@

             PYBIND11_TYPE_CASTER(type, handle_type_name<type>::name());
         };
+        template <class T>
+        struct numpy_traits
+        {
+        private:
+
+          constexpr static const int value_list[15] = {
+            NPY_BOOL,
+            NPY_BYTE,   NPY_UBYTE,   NPY_SHORT,      NPY_USHORT,
+            NPY_INT,    NPY_UINT,    NPY_LONGLONG,   NPY_ULONGLONG,
+            NPY_FLOAT,  NPY_DOUBLE,  NPY_LONGDOUBLE,
+            NPY_CFLOAT, NPY_CDOUBLE, NPY_CLONGDOUBLE
+          };
+
+        public:
+
+          using value_type = std::remove_const_t<T>;
+
+          static constexpr int type_num = value_list[pybind11::detail::is_fmt_numeric<value_type>::index];
+        };
+
     }
 }
SylvainCorlay commented 7 years ago

Thanks for the patch! I will try it out.

nbecker commented 7 years ago

I am working on a better patch, but I'm struggling with git. Basically, the same diff for pytensor and pyarray, and refactor the traits into it's own header

nbecker commented 7 years ago

https://github.com/QuantStack/xtensor-python/pull/75

I'm a git(hub) newb, so hopefully I did this OK

On Wed, Apr 12, 2017 at 9:37 AM Sylvain Corlay notifications@github.com wrote:

Thanks for the patch! I will try it out.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/QuantStack/xtensor-python/issues/73#issuecomment-293578904, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHK0ATpi0b46x9VghyNgOy29KgNGdy8ks5rvNOhgaJpZM4M6Qeg .

nbecker commented 7 years ago

Also, patch only addresses typenum mismatch, maybe there are other array flags that imply conversion that should be checked?

It would be nice if the numpy ABI had a function to check whether array can be imported without conversion, but I didn't see one.

wolfv commented 7 years ago

Fixed :)