Skip to content
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

cython magic failing to work with openmp. #2669

Closed
johntyree opened this issue Dec 8, 2012 · 14 comments · Fixed by #2691
Closed

cython magic failing to work with openmp. #2669

johntyree opened this issue Dec 8, 2012 · 14 comments · Fixed by #2691
Milestone

Comments

@johntyree
Copy link

Specifically I'm referring to the notebook. Apparently if any python function calls an openmp function, it fails. This example works fine in normal cython use, with the args put into setup.py and the extension imported into IPython manually.

Cell 1:

%load_ext cythonmagic
import Cython.Compiler.Options as CO
CO.extra_compile_args = ['-fopenmp']
CO.extra_link_args = ['-fopenmp']

Cell 2:

%%cython
cimport openmp
import cython.parallel as cp

def ompfunc():
    openmp.omp_set_dynamic(1)
    with nogil, cp.parallel():
        with gil:
            print cp.threadid()

It will build if you change ompfunc to a cdef, but of course the kernel then gives the following error:

/home/john/.ipython/cython/_cython_magic_1fbf0dc97ef57476220c1cdf9c600a49.c:1252:18:
    warning: ‘__pyx_f_46_cython_magic_1fbf0dc97ef57476220c1cdf9c600a49_ompfunc’
    defined but not used [-Wunused-function]
@bfroehle
Copy link
Contributor

bfroehle commented Dec 8, 2012

That's a warning, not an error. What do you mean by doesn't work?

@johntyree
Copy link
Author

Yes it's a warning and not an error, but that warning is because the function is cdef and not def, meaning it's not visible from python land and therefore is unused I guess. If it's not in python land, you have no way to call the function so it might as well have just failed to build. That would even be preferable to the runtime error you get.

@johntyree
Copy link
Author

As I originally said, the code as written, with def ompfunc():, fails.

ImportError: /home/john/.ipython/cython/_cython_magic_d97b646f1ec3c7e6350f3f6d2b3633b2.so: undefined symbol: omp_set_dynamic

Examining it more closely, I suppose it does build... it just can't be imported because it didn't resolve the omp symbols..

@bfroehle
Copy link
Contributor

bfroehle commented Dec 8, 2012

It seems that the %%cython magic doesn't use the Cython compiler options directives. There are some flags for setting those -c and -la, maybe you can try to use those to pass the -fopenmp flags?

I'll try to look at this more tomorrow. Thanks for the report and your investigation so far.

@dhirschfeld
Copy link

It works for me (on windows) if I pass in the openmp flags for both complie-args and link-args:

%%cython --compile-args=/openmp --link-args=/openmp --force
cimport cython
cimport openmp

import cython.parallel as cp
from cython.parallel import parallel, prange

import numpy as np
cimport numpy as np

ctypedef np.int32_t int32_t

cdef extern from "windows.h" nogil:
    void Sleep(int ms)

@cython.boundscheck(False)
cpdef double[::1] ompfunc(int32_t size, int32_t num_threads):
    cdef int32_t idx
    cdef double[::1] result = np.empty(size, dtype=np.float64)
    with nogil, cp.parallel(num_threads=num_threads):
        for idx in prange(size, schedule='dynamic'):
            result[idx] = cp.threadid()
            Sleep(1000)
    return result
In [66]: np.asarray(ompfunc(10, 10))

Out[66]: array([ 0.,  1.,  2.,  3.,  4.,  5.,  6.,  7.,  8.,  9.])
In [67]: %timeit ompfunc(10, 1)
             %timeit ompfunc(10, 2)
             %timeit ompfunc(10, 3)
             %timeit ompfunc(10, 4)
             %timeit ompfunc(10, 5)
             %timeit ompfunc(10, 6)
             %timeit ompfunc(10, 7)
             %timeit ompfunc(10, 8)
             %timeit ompfunc(10, 9)
             %timeit ompfunc(10, 10)

1 loops, best of 3: 10 s per loop
1 loops, best of 3: 5 s per loop
1 loops, best of 3: 4 s per loop
1 loops, best of 3: 3 s per loop
1 loops, best of 3: 2 s per loop
1 loops, best of 3: 2 s per loop
1 loops, best of 3: 2 s per loop
1 loops, best of 3: 2 s per loop
1 loops, best of 3: 2 s per loop
1 loops, best of 3: 1e+03 ms per loop

@johntyree
Copy link
Author

Can it be made to use the compiler options directives normally or do I need to type them all out in every cell?

@johntyree
Copy link
Author

I mean via patching, of course.

Basically is this trivial or does it require deep architectural changes that are NeverGoingToHappen® ?

@dhirschfeld
Copy link

I'd imagine it's possible and maybe not even too difficult. The relevant code is:

https://github.com/ipython/ipython/blob/master/IPython/extensions/cythonmagic.py#L151

I believe the R-magic pulls in variables from the notebook namespace to pass to R so (I think) you should be able to similarly pull in a Cython.Compiler.Options variable you declared earlier.

@johntyree
Copy link
Author

Great. I'll take a look.

@bfroehle
Copy link
Contributor

@johntyree Is the usage of Cython.Compiler.Options in your first cell documented anywhere? I'm just curious how you found out about it and why you expected it to work.

For those just joining, the test code suggested by @dhirschfeld will work in Linux with slight modifications:

%%cython --compile-args=-fopenmp --link-args=-fopenmp --force
cimport cython
cimport openmp

import cython.parallel as cp
from cython.parallel import parallel, prange

import numpy as np
cimport numpy as np

ctypedef np.int32_t int32_t

cdef extern from "unistd.h" nogil:
    unsigned int sleep(unsigned int seconds)

@cython.boundscheck(False)
cpdef double[::1] ompfunc(int32_t size, int32_t num_threads):
    cdef int32_t idx
    cdef double[::1] result = np.empty(size, dtype=np.float64)
    with nogil, cp.parallel(num_threads=num_threads):
        for idx in prange(size, schedule='dynamic'):
            result[idx] = cp.threadid()
            sleep(1)
    return result

Another option could be to add a -fopenmp flag to %%cython which would be aliased to the compile-args / link-args combo.

@johntyree
Copy link
Author

@bfroehle, actually, from this thread that you participated in on cython-users.

https://groups.google.com/forum/?fromgroups=#!topic/cython-users/LZcC92mGx94

I can't say whether or not it's supposed to be supported, but there's no documentation that anyone can seem to find, so I imagine not. It seemed like a reasonable thing to have and I assumed it was just a way to programmatically specify the arguments.

@bfroehle
Copy link
Contributor

Okay, so for the moment I think you're stuck with the --compile-args / --link-args combo (so sorry!). But let's keep this open to figure out a better way.

@johntyree
Copy link
Author

That's fine. They solve the problem. The rest is convenience.

@minrk
Copy link
Member

minrk commented Jul 6, 2013

closed by #2691

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants