pele-python / mcpele

Monte Carlo and parallel tempering routines built on the pele foundation
Other
20 stars 5 forks source link

copy and paste in wrapping for python #24

Closed kjs73 closed 10 years ago

kjs73 commented 10 years ago

Personally I am scared of and bad at copy and paste. Whenever I do it, I tend to introduce bugs that I have trouble to find later on.

In the part of the project where the c++ elements are wrapped for cython usage, so far I end up doing a lot of copy and paste. This is mainly because I have no idea of the involved language. But it is also because some quite long phrases and operations need to be repeated and duplicated many times. (For example, passing a c++ pele::Array on to cython/python or forwarding the function declarations, or getting a c-type variable from newptr via a cdef-variable and then returning it in python.)

Sometimes it feels like some of these operations could maybe be automated to avoid copy and paste bugs. Do you guys think there is any hope for that?

js850 commented 10 years ago

Yes. I was thinking the same thing. Some utility / convenience functions would be useful.

js850 commented 10 years ago

One very nice thing would be

cdef np_to_array(v):
    """return a pele array that wraps the data in a numpy array"""
    cdef np.ndarray[double, ndim=1] vnp = np.asarray(v, dtype=double).reshape(-1)
    return _pele.Array[double]( <double *> vnp.data, vnp.size )

also the opposite

cdef array_to_np(v):
    """copy a pele::Array into a numpy array"""
smcantab commented 10 years ago

Looks good, we would also need a better way to print arrays. Currently we are actually looping through the array and copy the data one by one, eg

    @cython.boundscheck(False)
    @cython.wraparound(False)
    def get_histogram(self):
        """return a histogram array"""
        cdef _pele.Array[double] histi = self.newptr.get_histogram()
        cdef double *histdata = histi.data()
        cdef np.ndarray[double, ndim=1, mode="c"] hist = np.zeros(histi.size())
        cdef size_t i
        for i in xrange(histi.size()):
            hist[i] = histdata[i]

        return hist

there has to be a 1-liner to do this, all I could find was using np.asarray I think, but I am not sure whether it's the right way to go here

js850 commented 10 years ago

I don't think there is a 1-liner to do this, but this is a perfect example of what @kjs73 was talking about before. we should implement a cython function

cdef np.ndarray[double] array_to_np(_pele::Array[double] v):
    """copy the data from a pele array into a new numpy array"""