MAINT: Rewrote scipy.ndimage.find_object function in cython

[aman-harness]

MAINT: Rewrote scipy.ndimage.find_object function in cython.

[rgommers]

@aman-iitj the test failures on TravisCI are real

[aman-harness]

@rgommers All the tests passed on my laptop. Even I checked the file in IPython and I am getting correct output for the failed test cases in travis. Have I missed something?

[rgommers]

I guess so. TravisCI runs 64-bit Linux. Maybe you're on another OS or bit-ness, and this is a 32/64-bit issue or a compiler dependent bug.

[rgommers]

@aman-iitj it would be good to write more informative commit messages, that use the prefixes and format described in http://docs.scipy.org/doc/numpy-dev/dev/gitwash/development_workflow.html#writing-the-commit-message

[aman-harness]

@rgommers Yes, I am working on 32 -bit system.
Thanks for the link. I will try to follow the commit message guidelines.

[jaimefrio]

Hi Aman,

Thanks for making this first PR. On a first scan of your code, I wasn't able to find anything obviously wrong that would lead to the test failures. But I did get the impression (which may be wrong) that it mimics too closely what the existing C implementation did, which makes it more complicated than could be achievable with Cython.

I'll try to turn that loose generic comment into actionable recommendations in the next few days: hopefully as we address those we can figure out where is the bug in your code.

[aman-harness]

Thanks for going through the code and suggestions @jaimefrio .
Regarding the error in tests, I have narrowed the code which is causing error and soon I will fix it.

[aman-harness]

@jaimefrio @rgommers TravisCI tests have passed :)

[aman-harness]

@jaimefrio Have you gone through the find_objects function. I have added 2 more functions using PointIterator in the PR. Please have a look at _ni_morphology.pyx file also.

[jaimefrio]

We shold also add the necessary configuration to the Bento build files.

[jaimefrio]

This is not being used anywhere right now. If there is an advantage in using this over ndarray.ndim lets do it, if not we can remove it from here.

[jaimefrio]

Try all the following together, I'm pretty sure it should work:

• get_funcs should not be a Python def function, but should be cdef with return type func_p *. This should make the casting of findObjectsPoint[data_t] unnecessary, remove it too.
• In _findObjects get rid of the funcs variable, and directly do:

cdef:
    func_p findObjectsPoint = get_funcs(input_raw.take([0]))

What errors are you getting when you try that?

[aman-harness]

@jaimefrio i declared get_funcs like:
cdef * funcp get_funcs(np.ndarray[data_t] input): return ( findObjectsPoint[data_t])
And I am getting error as:

inux-gnu-gcc' failed with exit status 1\n"]
In [40]: reload(_ni_measure)
Error compiling Cython file:
------------------------------------------------------------
...
#####################################################################
ctypedef void (*func_p)(void *data, np.flatiter iti, np.intp_t max_label, 
               int *regions, int rank) nogil
cdef * funcp get_funcs(np.ndarray[data_t] input):
    ^
------------------------------------------------------------
_ni_measure.pyx:49:5: Expected an identifier, found '*'
Error compiling Cython file:
------------------------------------------------------------
...
#####################################################################
ctypedef void (*func_p)(void *data, np.flatiter iti, np.intp_t max_label, 
               int *regions, int rank) nogil
cdef * funcp get_funcs(np.ndarray[data_t] input):
            ^
------------------------------------------------------------
_ni_measure.pyx:49:13: Syntax error in C variable declaration

[aman-harness]

If i declare it like this
cdef funcp get_funcs(np.ndarray[data_t] input): return ( findObjectsPoint[data_t])
Error is:

 In [41]: reload(_ni_measure)
Error compiling Cython file:
------------------------------------------------------------
...
#####################################################################
ctypedef void (*func_p)(void *data, np.flatiter iti, np.intp_t max_label, 
               int *regions, int rank) nogil
cdef funcp get_funcs(np.ndarray[data_t] input):
    ^
------------------------------------------------------------
_ni_measure.pyx:49:5: 'funcp' is not a type identifier
Error compiling Cython file:
------------------------------------------------------------
...
        np.flatiter _iti
        PyArrayIterObject *iti
        # func_p findObjectsPoint =    funcs
        func_p findObjectsPoint = get_funcs(input_raw.take([0]))
                                          ^
------------------------------------------------------------
_ni_measure.pyx:109:43: no suitable method found

[jaimefrio]

I missed an underscore, the type should be func_p, which is the typedef defined in the line before that one.

[aman-harness]

@jaimefrio Sorry for the silly mistake. I was sleepy yesterday.
But still this doesn't works.

In [6]: import _ni_measure
Error compiling Cython file:
------------------------------------------------------------
...
ctypedef void (*func_p)(void *data, np.flatiter iti, np.intp_t max_label, 
               int *regions, int rank) nogil
cdef func_p get_funcs(np.ndarray[data_t] input):
    return ( findObjectsPoint[data_t])
                            ^
------------------------------------------------------------
_ni_measure.pyx:50:29: Cannot assign type 'object (int8_t *, flatiter, int, int *, int)' to 'void (*)(void *, flatiter, int, int *, int) nogil'
Error compiling Cython file:
------------------------------------------------------------
...

[aman-harness]

In [8]: import _ni_measure
Error compiling Cython file:
------------------------------------------------------------
...
#####################################################################
ctypedef void (*func_p)(void *data, np.flatiter iti, np.intp_t max_label, 
               int *regions, int rank) nogil
cdef *func_p get_funcs(np.ndarray[data_t] input):
    ^
------------------------------------------------------------
_ni_measure.pyx:49:5: Expected an identifier, found '*'
Error compiling Cython file:
------------------------------------------------------------
...
#####################################################################
ctypedef void (*func_p)(void *data, np.flatiter iti, np.intp_t max_label, 
               int *regions, int rank) nogil
cdef *func_p get_funcs(np.ndarray[data_t] input):
            ^
------------------------------------------------------------
_ni_measure.pyx:49:13: Syntax error in C variable declaration

[jaimefrio]

The * goes after the type, i.e. func_p*, not *func_p. The other error is what we already discussed, about making those functions take a void *, then cast it to the right type inside the function. If data_t has to be part of the function signature we may have to add an np.ndarray[data_t] argument to those functions, just for selecting the right one.

[aman-harness]

@jaimefrio I have already tried all the combinations of casting and functions declarations but nothing is working except casting with <Py_intptr_t> or <int *>.

As you have suggested in function findObjectPoint I need to add np.ndarray input[data_t] for signatures if I change data_t * to void *. If I declare input as a python object in _findObjects, it asks me to remove GIL giving compilation error: Python type test not allowed without GIL and after removing GlL it gives weird results(None for every case.). If I declare it as cdef np.ndarray input = np.PyArray_FROM_OF(input_raw, flags) it compiles successfully but is giving segmentation fault for every test case.

[jaimefrio]

Missing space after =.

[aman-harness]

@jaimefrio It doesn't works. I am still getting same errors.

[aman-harness]

@jaimefrio Even If I change the declaration of function as:

cpdef findObjectsPoint():
    cdef np.intp_t s_index = - 1
    
    return s_index

and I call it using

while np.PyArray_ITER_NOTDONE(_iti):

findObjectsPoint()

np.PyArray_ITER_NEXT(_iti)

the file doesn't compiles. It gives the same error.

Should I remove the function findObjectspoint() and update the array regions in _findObjects() itself?

[aman-harness]

@jaimefrio. How we are proceeding on it? Please have a look at my previous comments.

[ev-br]

Jaime @jaimefrio where does this PR stand in your ongoing ndimage refactor --- something left here, or can it be closed?