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-level interface #130

Open
jmschrei opened this issue Jun 12, 2017 · 7 comments
Open

Cython-level interface #130

jmschrei opened this issue Jun 12, 2017 · 7 comments
Labels
cat:feature New features/APIs

Comments

@jmschrei
Copy link

Howdy!

I'm currently trying to add CuPy support to one of my packages, and it would be very useful if there was a cython level interface. Since I'd mostly like to replace BLAS with GPU support, I was wondering if there were any plans to support a cython level API similar to the API that scipy supports for BLAS. Thanks!

@jekbradbury
Copy link
Contributor

This is the low-level Cython cuBLAS wrapper https://github.com/cupy/cupy/blob/master/cupy/cuda/cublas.pyx -- it’s mostly used towards the bottom of https://github.com/cupy/cupy/blob/master/cupy/core/core.pyx to implement cupy.dot, cupy.tensordot, and cupy.matmul but I think it’s still exported.

@yuyu2172
Copy link
Contributor

yuyu2172 commented Jun 22, 2017

@jmschrei

As @jekbradbury pointed out, cuBLAS wrapper functions are exported at Cython level.
However, many of the NumPy compatible functions are either not cpdef functions or cpdef functions not exported outside.

I played around a bit, and it was relatively simple to add cpdef functions to pxd files. By doing that, projects outside of CuPy can call them at Cython level.
I created a simple demo project that demonstrates such usage.
https://github.com/yuyu2172/cupy-cython

Edit:
I meant cdef or cpdef.

@jmschrei
Copy link
Author

Howdy

Thanks for the responses. I've been out for a bit, but I'm looking into this more. When I said I wanted a cython function, I meant that I was interested in a drop-in replacement for dgemm. Essentially, this is what my code looks like now in a function that has the GIL released:

if GPU[0] == 1:
	with gil:
		x = ndarray_wrap_cpointer(X, n*d).reshape(n, d)
		dot_ndarray = cupy.dot(x, self.inv_cov)
		dot = <double*> (<numpy.ndarray> dot_ndarray).data
else:
	dot = <double*> calloc(n*d, sizeof(double))
	mdot(X, self._inv_cov, dot, n, d, d)

What I would ideally like is something like:

if GPU[0] == 1:
    gpu_mdot(X, self.inv_cov, dot, n, d, d)
else:
    mdot(X, self.inv_cov, dot, n, d, d)

In my code, mdot is a wrapper for dgemm, and I'd like for gpu_mdot to also be some wrapper of cublasDgemm. I'm unsure what to pass in for the handle though. Can you point m to the appropriate usage of that?

@okuta
Copy link
Member

okuta commented Sep 17, 2017

This issue relates #79.
I agree about open Cython-level interface for users.
But, current CuPy Cython-level interface is unstable and non-documented.
Cython has no access control. If we open it, user may use all method that includes internal method.

Do you have good idea?

@okuta okuta added cat:feature New features/APIs cat:install Build and installation labels Sep 17, 2017
@honnibal
Copy link

But, current CuPy Cython-level interface is unstable and non-documented.
Cython has no access control. If we open it, user may use all method that includes internal method.
Do you have good idea?

I think if I'm making Cython-level calls into your library, then compatibility and stability should be my problem, not yours :)

At the C-level the semantic versioning really breaks down. The .pxd files often end up including code that isn't part of the interface. For instance, any inline method needs to be in the .pxd. The same is true of static methods.

One policy could be that if my library depends on your C structures, I should pin a range up to an exact version. It's then up to me to test your new versions and adjust my pin.

Another policy could be that if you change the .pxd files, that should be versioned as a minor release. I think this would help a lot in preventing version lock, but if it make things too hard for you, I'm happy to pin to an exact patch.

@sonots
Copy link
Contributor

sonots commented Nov 10, 2017

It looks this guy also wants cython interface of CuPy https://stackoverflow.com/questions/46510663/will-cupy-support-cython-eg-buffered-index.

@jakirkham
Copy link
Member

But, current CuPy Cython-level interface is unstable and non-documented.

What about starting with a big scary note in the docs that says exactly this? 😉

Seriously though I think this would be really nice for other Cython libraries that use CuPy. Hopefully over time we can come to a consensus about what things deserve to be public and stable, but it is likely something that will require a fair bit of iteration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat:feature New features/APIs
Projects
None yet
Development

No branches or pull requests

8 participants