Updating collections

[thomasdeneux]

Hello, I don't find how to update basic values of collections.

For example, i can create a points collection:

points = PointCollection("agg", color="shared", size="local")
n = 1000
self.points.append(np.random.normal(0.0, 0.5, (self.n, 3))
    color=np.column_stack((np.random.random((self.n, 3)), np.ones(self.n))),
    size=8)

But trying to change its vertices coordinates:

points['position'] = np.random.normal(0.0, 0.5, (self.n, 3))

Gives me the error:

File "/AlphaI/test.py", line 36, in change
  self.points['position'] = pos
File "/vispy/vispy/visuals/collections/collection.py", line 192, in __setitem__
  BaseCollection.__setitem__(self, key, value)
File "/vispy/vispy/visuals/collections/base_collection.py", line 432, in __setitem__
  V[key] = data
File "/vispy/vispy/gloo/buffer.py", line 271, in __setitem__
  raise ValueError("Cannot set non-contiguous data on buffer")

Besides, I managed to update the colors:

points['color'] = np.column_stack((np.random.random((self.n, 3)), np.ones(self.n)))
canvas.update()  # this involves redrawing the points

This was possible only with color="shared" when constructing the PointCollection object, while color="local" resulted in the same error as above when trying to update the color; but i could not guess appropriately what these keywords mean. I tried a similar position="shared" but this was not accepted. In general i am suffering from the lack of documentation!

[djhoese]

@rougier Any advice?

[thomasdeneux]

but i could not guess appropriately what these keywords mean

Of course i understand this is about different types of declarations in the shaders. I am not very familiar with those yet.

[rougier]

This should work, I don't see where's the problem. Can you check what is the code at line 271 on gloo/buffer ?

[thomasdeneux]

@rougier First sorry for the delay, i just was en vacances.

So, no it clearly does not work! I show more code below, this is the latest version of the vispy master branch.

It appears that in the BaseCollection.__setitem__ method there is a call for setting the vertices buffer using a string key (which in my case is 'position'):

403    def __setitem__(self, key, data):
           (...)

420        V = self._vertices_buffer
           (...)

428        # Setting a whole field
429        if isinstance(key, str):
430            # Setting a named field in vertices
431            if key in self.vtype.names:
432                V[key] = data
               (...)

But writing in a DataBuffer with a string key is actually not handled:

265    def __setitem__(self, key, data):
266        """ Set data (deferred operation) """
267
268        # Setting a whole field of the buffer: only allowed if we have CPU
269        # storage. Note this case (key is string) only happen with base buffer
270        if isinstance(key, string_types):
271            raise ValueError("Cannot set non-contiguous data on buffer")
           (...)

My understanding is that each vertex of this points collection has 'position', 'size' and 'collection_index' (by the way, does this one handle the color?) fields, and it is forbidden to set only one field as this represents non-contiguous data in the memory?

Note also that V is a VertexBuffer but this class does not have a __setitem__ method, so it is the method of the DataBuffer parent class that is called, is it the expected behavior?

[thomasdeneux]

Note also that i have the same error for other collection objects, e.g. lines:

  File "E:/Users/Thomas/Python/Naivia/AlphaI/test.py", line 34, in change
    self.lines['P0'] = pos[0::2]
  File "e:\users\thomas\python\external\vispy\vispy\visuals\collections\collection.py", line 192, in __setitem__
    BaseCollection.__setitem__(self, key, value)
  File "e:\users\thomas\python\external\vispy\vispy\visuals\collections\base_collection.py", line 432, in __setitem__
    V[key] = data
  File "e:\users\thomas\python\external\vispy\vispy\gloo\buffer.py", line 271, in __setitem__
    raise ValueError("Cannot set non-contiguous data on buffer")
ValueError: Cannot set non-contiguous data on buffer

[rougier]

Ouch. That may be a structural difference between vispy and glumpy. I originally coded collections for glumpy and we ported them to vispy. In glumpy, I compute the smallest contiguous chunk of memory to uploadi it at once to the GPU. In vispy, I don't quite remember how we coded the data buffer but it might be possible we forbid such update. To solve the problem would thus require to (maybe) rething the data buffer a little bit. If it is not clear, there's also an explanation at https://www.labri.fr/perso/nrougier/from-python-to-numpy/#memory-aware-array

[thomasdeneux]

Thank you for the explanation (and nice book by the way).
Well, yesterday, as i was blocked with the collections, i started writing my own shaders to use gloo directly: so i will continue like this!
Otherwise my understanding is that, in order to reproduce the mechanisms present in glumpy, vispy buffer should also have a CPU counterpart where to write in 'non-contiguous' way, while only continuous chunks will be later uploaded to the GPU (either immediately after the write, or only before this data is actually needed on the GPU, which would require even more work). Am i right?

[rougier]

Yes, for vispy I think we decided at some point to not have a CPU image and this prevent us to to upload non contiguous data to a GPU buffer from what I remember. For glumpy, I chose a different option, the advantage being to be able to upload non contiguous data and to batch all updates in one operation.

One potential solution for vispy and collection would be to split the single GPU buffer into several one but this may impact performances.

[thomasdeneux]

Oh, i can't go down to write my own shaders as i would like to draw antialiased lines, this would be too difficult!

Maybe do i start to understand enough to try editing myself the buffer classes. What about having an optional CPU buffer (default would be not to have it), which, if activated, makes it possible to write non-contiguous data? Do you see any difficulty with this?

[rougier]

I'm not sure how much work is needed. but you can have a look at glumpy to check for the implementation of the CPU implementation and see differences with vispy.

[djhoese]

@rougier The disadvantages of having a CPU backed buffer were mentioned so just wanted to make sure I'm understanding this properly.

1. Current VisPy: Require the entire buffer data to be sent at once. Benefit is that the data doesn't have to be kept in CPU-memory because it "lives" on the GPU only. Downside as discussed is that you can't send per-vertex updates without having all of the vertex's properties. For a VertexBuffer this is position by default but in the case of how @thomasdeneux is using it the size is also used, at least from what I can tell.

If I'm understanding the existing vispy version of collections and this issue, we don't necessarily need to have the entire buffer's data to send the individual vertex update, we just need to have a single "dtype"s data to send the right "block" of data.

2. Glumpy: Holds on to a copy of the buffer data on the CPU side so that any partial data updates can be "filled" in by the CPU side data and pushed to the right "block" on the GPU side. Downside being that the data has to exist on the CPU and GPU. Upside being that you can actually do these partial data updates.

@rougier Does that sound right at all? Could you point us at the relevant functions in glumpy's versions of the buffers? iirc you thought that vispy might be used by glumpy in a scipy:numpy-like relationship. If we want this to happen at some point in the future we'll need to resolve these inconsistencies.

It doesn't sound like it is impossible to have both of these options and it doesn't sound like the user shouldn't be able to configure them if they are worried about all levels of performance.

[rougier]

You're right. The other problem with the GPU only version is that all pending upload operations cannot be agglomerated into a single operation. If you have many operations, this can slow down things a lot.

But for very large buffers, I don't know if the glumpy approach is good. We would need some benchmarks at some point.

Concerning the scipy/numpy relationship, you're right. It is the way I generally present glumpy and vispy . I did not implement the vispy scene stuff which is a real strength of vispy. But it would make sense to have two packages (and possibly later, some toolkits). However, the other problem is also the shader variables handling that may be not compatible at all. @campagnola @almarklein @rossant what do you think / remember of these choices ?

[thomasdeneux]

@rougier Hello, I am back on this issue. I had a look at how Buffers are implemented in glumpy, I see that they inherit from GPUdata, which itself inherits from np.ndarray and provides mechanisms for keeping track of the memory range that has been modified.

So please find here how I propose to implement CPU buffers in vispy and please comment and/or give alternative suggestions:

• I copy the code of this glumpy GPUData into vispy. Yet wouldn't it make more sense to call it CPUData?! Indeed, it only takes care of data that is in CPU memory.
• It seems reasonable that using such local CPU buffer for vispy Buffer objects should be optional. Therefore vispy Buffer cannot inherit from GPUData (or CPUData if you agree to change the name), rather there should be a new attribute local_data that will be used or not depending on an optional use_cpu_buffer argument to the constructor, by default equal to False.
• Rather than creating new methods to the Buffer class for modifying this local_data, i suggest that it will be a public property that can be modified directly. Of course an error would be thrown if one attempts to modify this property when use_cpu_buffer is False.
• I imagine an additional optional constructor argument immediate_upload, by default equal to True determining whether modifications of the buffer will be immediately uploaded to GPU or whether an explicit call to a new upload method is necessary. It is not clear to me, when immediate_upload will be set to False, whether existing methods set_data and set_subdata should still upload immediately or not.
• Then the DataBuffer and VertexBuffer classes will also have a similar use_cpu_buffer and immediate_upload optional arguments which, when the first will be set to True, will allow writing only some attributes (e.g. position).
• What about the DataBufferView and IndexBuffer classes? I did not understand them enough to know if they will also need optional CPU buffer.
• Finally, I feel that the collection classes should create their vertex buffers with use_cpu_buffer=True. Regarding immediate_upload i will start with value True, and later see whether it can optionally be set to False, while a additional flushing method will be needed.

How does this sound?

[rougier]

Yes, that sounds like a reasonable approach. Instead of use_cpu_buffer, I would use cpu_cpu. For the GPU / CPU, you might be right that CPU might be a better name in the vispy context.

For the DataBufferView, I can't find where it is defined in glumpy, do you know?

[thomasdeneux]

Thanks. You mean use_cpu right?

I don't know anything about glumpy except now for this Buffer and GPUData classes i studied, so i can't answer your question!

[rougier]

Actually I meant cpu_copy

[rougier]

Oh, DataBufferView is part of vispy, is it ?

[thomasdeneux]

Indeed! My question was: what should i do with the vispy DataBufferView class, should i just set use_cpu to False in the constructor (which as noted in a comment is an "evil" constructor that doesn't call its superclass constructor), or will it need the CPU buffer in some situations?

And similar question for the IndexBuffer class: does it need use_cpu as a constructor argument, or do i just call the DataBuffer super-class without this argument (i.e. the default value False will be set)?

[rougier]

I think you'll need it for the index buffer but probably not for he data buffer view (since it is merely a view on a data buffer that has been already declared with use_cpu or not)

[thomasdeneux]

ARGHH!! I have finally made this feature of an optional CPU buffer (see PR #1538), but Collections still have a log of other bugs. Now i do not get an error any more when setting, for example points['position'], but the display does not update! In fact, only updating the 'color' works as expected, but the position or size of agg points, or the edge positions or line width of agg lines do not update. Some preliminary debugging suggests me that the collection _vertex_list property overwrites the changes that i am attempting to do to the vertex buffer, but it doesn't seem easy to fix it.

I have a demo of my program scheduled Friday for which i need fast updates (as opposed to the slow updates in the current stable version that uses individual Scenegraph points and lines). I won't pursue now with collections, and rather return to my alternative path of writing my own shaders. But i would appreciate if @rougier you can comment.

[rougier]

Where is this _vertex_list?

[thomasdeneux]

It is an attribute of the BaseCollection class. I have the impression that the Collection.draw method systematically enters its if self._need_update: self._update(), and that this results in the vertex buffer to be overwritten by the data inside BaseCollection._vertex_list attribute. But I may be wrong.

Better is to give an example, but for it to not give execution error, you need to run it on top of PR #1538. You will see that when method change is called after 2s, only the color of points changes on display, but not the other points and lines properties.

import numpy as np
from vispy import app, gloo
from vispy.visuals.collections import PointCollection, SegmentCollection


class MyCanvas(app.Canvas):
    def __init__(self):
        super(MyCanvas, self).__init__()

        self.points = PointCollection("agg", color="shared", size="local")
        self.n = 10
        pos = 1-2*np.random.random((self.n, 2))
        self.points.append(np.column_stack((pos, np.zeros(self.n))),
                           color=np.column_stack((np.random.random(
                               (self.n, 3)), np.ones(self.n))),
                           size=np.random.random(self.n)*20)

        line_pos = np.column_stack((pos, -np.ones(self.n)))
        self.lines = SegmentCollection("agg", linewidth="local")
        self.lines.append(line_pos[0::2], line_pos[1::2],
                          linewidth=np.random.random(self.n//2)*10)

    def change(self, event):
        print('change properties')
        # change points positions, colors and size
        pos = 1-2*np.random.random((self.n, 3))
        color = np.column_stack(
            (np.random.random((self.n, 3)), np.ones(self.n)))
        x = self.points._vertices_buffer[:]
        self.points['position'] = pos
        self.points['color'] = color
        self.points['size'] = np.random.random(self.n)*20
        # change lines positions and widths
        self.lines['P0'] = np.repeat(pos[0::2], 4, axis=0)
        self.lines['P1'] = np.repeat(pos[1::2], 4, axis=0)
        self.lines['linewidth'] = np.repeat(np.random.random(self.n//2)*10, 4)
        self.update()

    def show(self):
        super(MyCanvas, self).show()
        gloo.set_state("translucent",
                       depth_test=False)  # not sure what this does

    def on_draw(self, event):
        gloo.clear('white')
        self.points.draw()
        self.lines.draw()

    def on_resize(self, event):
        width, height = event.size
        self.context.set_viewport(0, 0, width, height)
        self.lines['viewport'] = (0, 0, width, height)


C = MyCanvas()
C.show()
t = app.Timer(interval=2, start=True, connect=C.change, iterations=1)
app.run()

[rougier]

Note that color="shared", size="local" means color is stored in the accompanying texture while size is stored in the buffer, that may explain the behavior being different.

[thomasdeneux]

sure, but size has to be local so i see no way to successfully change the property
(note in any case that i switched to not using collections, so this issue is no longer important for me)