-1

My question is similar to the one asked here - Passing C++ vector to Numpy through Cython without copying and taking care of memory management automatically I am also getting a segmentation fault but before I can fix it I have some compilation errors to take of with the move method in Cython.

This is the simplest example I have (just an extension of the Rectangle example provided in the net

What I want to do My C++ code returns a deque of Points. It can return a vector of points as well. Any container will do

In the cython code I want to store the returned deque of points(one for each iteration of the for loop) in a collection(such as vector or deque). This collection will be an instance variable.

Later on I want to iterate through the collection and convert that deque of deque of points into a python list of lists. Here is where I believe I am getting the segmentation fault.

Point.h

#ifndef POINT_H
#define POINT_H

class  Point
{
  private:
  double coordinate1,coordinate2;
  public:
   virtual double  getCoordinate1() const;
   virtual double  getCoordinate2() const ;
   virtual void    setCoordinate1(double coordinate1);
  virtual void    setCoordinate2(double coordinate2);
 };

Rectangle.h

   #include <deque>
   #include "Point.h"
   using std:deque;

   deque<Point> getAllPoints(Point query);

Rectangle.cpp

    include "Rectangle.h"

     deque<Point> Rectangle::getAllPoints(Point query)
     {
      deque<Point> deq;
        for (int i = 0;i < 10000; ++i)
         {
           deq.push_back(query);
         }
      return deq;

Note Unlike the linked question I am not returning an address but returning a reference

rect.pxd

 cdef extern from "<utility>" namespace "std" nogil:
   T move[T](T) #
 cdef extern from "Point.h":
   cdef cppclass Point:
   Point() nogil except +
   double getCoordinate1()
   double getCoordinate2()
   void setCoordinate1(double coordinate1) nogil
   void setCoordinate2(double coordinate2) nogil

cdef cppclass SphericalPoint(Point):
   SphericalPoint() nogil except +
   double getCoordinate1()
   double getCoordinate2()
   void setCoordinate1(double lat) nogil
   void setCoordinate2(double lon) nogil

cdef extern from "Rectangle.h" namespace "shapes":
   cdef cppclass Rectangle:
     Rectangle(int, int, int, int) except + nogil
     deque[Point] getArea(Point p) nogil

And finally

rect.pyx 

 cdef class PyRectangle:
   cdef Rectangle *rect    
   cdef deque[Point] colOfPoints

   def __cinit__(self, int x0, int y0, int x1, int y1):
    self.rect = new Rectangle(x0, y0, x1, y1)
    self.colOfPoints = deque[Point]()

 def performCalc(self,maxDistance,chunk):
    cdef deque[Point] area
    cdef double[:,:] gPoints
    gPoints = memoryview(chunk)
    for i in range(0,len(gPoints)):
        with nogil:
            area =  self.getArea(gPoints[i])
            self.colOfPoints = move(area)

cdef deque[Point] getArea(self,double[:] p) nogil:
    cdef deque[Point] area
    area = self.rect.getArea(point)
    return area

I believe I am setting the C++ 17 in setup.pyx

setup.py

  os.environ['CFLAGS'] = '-O3 -Wall -std=c++17'

  ext_modules = [Extension("rect",
                     ["rect.pyx","Rectangle.cpp"],
                     include_dirs=['/usr/local/include'],
                     extra_link_args=["-std=c++17"],
                     language='c++',
                 )]

extensions = cythonize(ext_modules, language_level = "3")

I am getting these compilation errors

 rect.cpp: In function ‘PyObject*   __pyx_pf_4rect_11PyRectangle_6performCalc(__pyx_obj_4rect_PyRectangle*, PyObject*, PyObject*)’:
 rect.cpp:3781:81: error: no matching function for call to ‘move<std::deque<Point, std::allocator<Point> > >(std::deque<Point>&)’
       __pyx_v_self->colOfPoints = std::move<std::deque<Point> >(__pyx_v_area);
                                                                             ^
     In file included from /usr/include/c++/7 /bits/nested_exception.h:40:0,
             from /usr/include/c++/7/exception:143,
             from /usr/include/c++/7/ios:39,
             from rect.cpp:632:
   /usr/include/c++/7/bits/move.h:98:5: note: candidate: template<class _Tp> constexpr typename std::remove_reference< <template-parameter-1-1> >::type&& std::move(_Tp&&)
 move(_Tp&& __t) noexcept
 ^~~~
 /usr/include/c++/7/bits/move.h:98:5: note:   template argument deduction/substitution failed:
  rect.cpp:3781:81: note:   cannot convert ‘__pyx_v_area’ (type ‘std::deque<Point>’) to type ‘std::deque<Point>&&’
       __pyx_v_self->colOfPoints = std::move<std::deque<Point> >(__pyx_v_area);
                                                                             ^
 In file included from /usr/include/c++/7/bits/char_traits.h:39:0,
             from /usr/include/c++/7/ios:40,
             from rect.cpp:632:
 /usr/include/c++/7/bits/stl_algobase.h:479:5: note: candidate: template<class _II, class _OI> _OI std::move(_II, _II, _OI)
 move(_II __first, _II __last, _OI __result)
 ^~~~
 /usr/include/c++/7/bits/stl_algobase.h:479:5: note:   template argument deduction/substitution failed:
    rect.cpp:3781:81: note:   candidate expects 3 arguments, 1 provided
       __pyx_v_self->colOfPoints = std::move<std::deque<Point> >(__pyx_v_area);
                                                                                    ^
   In file included from /usr/include/c++/7/deque:66:0,
             from rect.cpp:636:
   /usr/include/c++/7/bits/deque.tcc:1048:5: note: candidate: template<class _Tp> std::_Deque_iterator<_Tp, _Tp&, _Tp*> std::move(std::_Deque_iterator<_Tp, const _Tp&, const _Tp*>, std::_Deque_iterator<_Tp, const _Tp&, const _Tp*>, std::_Deque_iterator<_Tp, _Tp&, _Tp*>)
 move(_Deque_iterator<_Tp, const _Tp&, const _Tp*> __first,
 ^~~~
   /usr/include/c++/7/bits/deque.tcc:1048:5: note:   template argument deduction/substitution failed:
  rect.cpp:3781:81: note:   candidate expects 3 arguments, 1 provided
       __pyx_v_self->colOfPoints = std::move<std::deque<Point> >(__pyx_v_area);
                                                                             ^
   In file included from /usr/include/c++/7/deque:64:0,
             from rect.cpp:636:
  /usr/include/c++/7/bits/stl_deque.h:424:5: note: candidate: template<class _Tp> std::_Deque_iterator<_Tp, _Tp&, _Tp*> std::move(std::_Deque_iterator<_Tp, _Tp&, _Tp*>, std::_Deque_iterator<_Tp, _Tp&, _Tp*>, std::_Deque_iterator<_Tp, _Tp&, _Tp*>)
 move(_Deque_iterator<_Tp, _Tp&, _Tp*> __first,
 ^~~~
  /usr/include/c++/7/bits/stl_deque.h:424:5: note:   template argument deduction/substitution failed:
  rect.cpp:3781:81: note:   candidate expects 3 arguments, 1 provided
       __pyx_v_self->colOfPoints = std::move<std::deque<Point> >(__pyx_v_area);
gansub
  • 1,164
  • 3
  • 20
  • 47
  • So just to clarify the [mre] issue: we don't know what's in "Point.h". There's also a few template typos (e.g. `deque>` - count the <>...) which suggests this isn't quite the code that generates the error. I think I know what the issue is, so I'll answer once I've tested it (if it is the issue) – DavidW Aug 04 '19 at 08:34
  • @DavidW yes fixed the deque issue as well !! Sorry about that ! – gansub Aug 04 '19 at 08:36

1 Answers1

1

Below is the slightly simplifier version that I used to reproduce your issue. I'm just including it as an illustration of how to cut your example down further - note that I don't need to use C++ files - I can just include the code directly in the pyx file by putting it in the docstring.

#distutils: language = c++

from libcpp.deque cimport deque

cdef extern from *:
    """
    #include <deque>
    using std::deque;

    class Point{};

    deque<Point> getAllPoints() {
        deque<Point> deq;
        for (int i=0; i<10000; ++i) {
            deq.push_back(Point{});
        }
        return deq;
    }

    """
    cdef cppclass Point:
        pass
    deque[Point] getAllPoints()

cdef extern from "<utility>" namespace "std" nogil:
    T move[T](T)

cdef class PyRectange:
    cdef deque[Point] colOfPoints

    def something(self):
        cdef deque[Point] area = self.getArea()
        self.colOfPoints = move(area)

    cdef deque[Point] getArea(self):
        return getAllPoints()

The basic problem is that when Cython generates C++ code for templates it writes std::move<deque<Point>>(area) rather that std::move(area) and letting C++ deduce the template type. For reasons I don't fully understand this seems to generate the wrong code a lot of the time.

I have two and a half solutions:

  1. Don't tell Cython that move is a template function. Instead just tell it about the overloads you want:

    cdef extern from "<utility>" namespace "std" nogil:
    deque[Point] move(deque[Point])

    This is easiest I think and probably my approach.

  2. If you avoid creating the temporary area then the C++ code does work with the template:

    self.colOfPoints = move(self.getArea())

    I suspect in this case you might not even need the move - C++ will probably automatically use the move assignment operator anyway.

  3. This answer claimed to come up with a small wrapper helps Cython call the correct move (it also slightly missed the actually problem on the question it was answering...). I haven't tested it myself, but it might be worth a go if you want a templated move.

DavidW
  • 29,336
  • 6
  • 55
  • 86
  • 1
    yes thanks. I will be sure to check it out. I asked this question on meta and any suggestions will be appreciated - https://meta.stackoverflow.com/questions/388123/what-is-a-minimum-reproducible-verifiable-example-for-cython – gansub Aug 04 '19 at 08:54
  • 2
    OK... Don't worry too much about downvotes on meta - they don't count! – DavidW Aug 04 '19 at 08:55
  • not sure if you are interested but I posted this question on cython google groups - https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!msg/cython-users/AEyflJYLblI/ZDRUpnS-BAAJ because I felt it became discussion oriented for SO. I would eventually love to ask to here it as well but gist of it is I am unable to iterate after the move operation. – gansub Aug 23 '19 at 03:27
  • my guess is that I am going to need a std:move_iterator rather than a std::move. If that is the case then this has not been asked on SO yet i.e. how to use a move_iterator with Cython and may be a valid question providing I can crop the excess material in the question – gansub Aug 23 '19 at 03:45
  • Essentially what Robert is saying in the Cython google group posting is the gist of this SO answer - https://stackoverflow.com/questions/15704565/efficient-way-to-return-a-stdvector-in-c i.e. Return Value Optimization. The compiler moves it without the move. If that is the case not sure why it does not work for me – gansub Aug 23 '19 at 04:25
  • I don't really want to get to sign up to the Google groups discussion but: look _very_ carefully at your iterator increments to make sure you have the right one. I think you have an infinite loop. – DavidW Aug 23 '19 at 06:31