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

Image textures backed by cupy array #2391

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

brisvag
Copy link
Collaborator

@brisvag brisvag commented Sep 29, 2022

This PR is a very rough example of the kind of changes needed to allow cupy arrays to be preserved all the way to the GL level. (see #1985, #1986, cupy/cupy#5711, napari/napari#2243).

The changes on the Texture and Image level are overall quite simple, but things get problematic at lower levels where numpy interals are used in ways that I don't quite understand yet.

If one runs the new example (which is a copy of the existing image example but converting the data to a cupy array), the following exception is thrown:

  [...]
  File "/home/lorenzo/git/vispy/vispy/gloo/glir.py", line 822, in parse
    self._parse(command)
  File "/home/lorenzo/git/vispy/vispy/gloo/glir.py", line 792, in _parse
    ob.set_data(*args)
  File "/home/lorenzo/git/vispy/vispy/gloo/glir.py", line 1551, in set_data
    gl.glTexSubImage2D(self._target, 0, x, y, format, gtype, data)
  File "/home/lorenzo/git/vispy/vispy/gloo/gl/_gl2.py", line 1130, in glTexSubImage2D
    pixels = pixels_.ctypes.data
AttributeError: 'ndarray' object has no attribute 'ctypes'

The code where this error happens is apparently autogenerated, but examply how I'm not sure. Hopefully the git blamed @almarklein can point us in the right direction here :)

cc @haesleinhuepf @jakirkham

@brisvag brisvag changed the title do not mess up cupy arrays Image textures backed by cupy array Sep 29, 2022
@almarklein
Copy link
Member

If one runs the new example (which is a copy of the existing image example but converting the data to a cupy array), the following exception is thrown

Numpy arrays have a ctypes attribute through which the underlying memory can be accessed. I guess this works differently in cupy, so it'd need a triage there as well, I suppose :)

@brisvag
Copy link
Collaborator Author

brisvag commented Sep 29, 2022

Yup, I think this is what this example is showcasing, which we need to adapt here. My question was mainly: where/how is this code generated, and can/should we use the same machinery for generating similar code for cupy?

EDIT: sorry, just realized it says so in the docstring :P

@djhoese
Copy link
Member

djhoese commented Sep 29, 2022

I have a local copy of that example where I started to try to convert it to vispy gloo. I ran out of time to dive really far into it, but remember getting stuck at a point where the user-level has a gloo object, but the user needed to tell Cupy the low-level OpenGL identifier (or maybe vice versa). We should have talked about this at the meeting. Shoot.

@brisvag
Copy link
Collaborator Author

brisvag commented Sep 29, 2022

Next time we can talk about it :P After playing around with this, my feeling is that we really need a relatively high level "split" for this to work well. Kinda like in the example, we probably should straight up use a different CupyBuffer object or something like that. There are several things that otherwise make no sense at the GL level, such as updating a subtexture using GL commands, which is what fails in the example in this PR.

@jakirkham
Copy link

Thanks for working on this @brisvag! 🙏

cc @leofang @trxcllnt @quasiben (as this relates to conversations we have had here (on VisPy) & CuPy)

@almarklein
Copy link
Member

Looks like most references (8x) to .ctypes are in this module:
https://github.com/vispy/vispy/blob/main/codegen/annotations.py

These all reference .ctypes.data. Might be beneficial to create a little helper function to avoid a triage on each occurrence.

There's also a reference here, where a pointer to the data is obtained instead:

" values = values_.ctypes.data_as(ctypes.POINTER(ctypes.c_float))"

@xloem
Copy link

xloem commented Nov 8, 2022

To clarify what I think has been touched on, I don't believe it is supported to form a GL buffer from a CUDA buffer without downloading and uploading the data [EDIT: this could be done as an on-device copy though]. What is supported is to form a CUDA buffer from a GL buffer. Then the two magically refer to the same data.

Unfortunately, the GL handle is behind the GLIR, which means either doing the CUDA work on the GLIR server, or breaking the normal GLIR boundaries for the special case of local, live execution. If the CUDA operations are performed client-side, then either the GLIR would be probed to access the handle, or the buffer would be made outside of the GLIR and the handle uploaded to load it.

That final option seems likely to meet the various usecases the most simply, although it's a little messy. This means the data would already have been uploaded and there would be no need to access it with ctypes at all.

For CuPy specifically, there are existing examples. Here is where an OpenGL handle is converted to a CUDA resource: https://gist.github.com/keckj/e37d312128eac8c5fca790ce1e7fc437#file-cupy_gl_interop-py-L89

And then imported as a cupy cuda memory pointer: https://gist.github.com/keckj/e37d312128eac8c5fca790ce1e7fc437#file-cupy_gl_interop-py-L117

It seems when interoperating between opengl and cuda, basically all the cuda tensors are allocated in opengl first, and then exported to cuda.

EDIT:
Ideas for the other options:

  • Raw handles could be used for ids when running locally. Maybe easiest if some GL is used clientside. This would solve everything.
  • Buffers could be copied device-side using CUDA behind the GLIR to simply assign one buffer to another. The cuda resource handle would be sent over the GLIR. This might be the easiest to implement, but introduces a device-side copy delay for every update. The delay may be very small given final rendering is not that much data.

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 this pull request may close these issues.

None yet

5 participants