Separate UI and processing

[ly29]

What

To introduce a split between code that updates sockets and the code that process data.

Why

To solve these issues:

• To be able to turn of processing while editing
• Simplify code
• Error handling
• Node states
• Process large data sets

How

Changed

The update function cannot be called update because blender calls the update function. (see comments)
The proposal is to call it sv_update.

The new def update(self): function has the following responsibilities:

• Decide if the node is ready
• Handle socket changes

For a simple node like Circle it could become like this.

def update(self):
   if 'Last Socket in init' in self.outputs:
      self.state='ready'

For multi in socket it could be like this.

def update(self):
   if not 'Last Socket in init' in self.outputs:
      self.state='not_ready'

   multi_socket(self,min=1)
   inputsocketname = 'data'
   outputsocketname = ['data']
   changable_sockets(self, inputsocketname, outputsocketname)
   count = 0 
   for socket in self.inputs:
      if socket.links:
          count += 1
   if count>sufficent:
       self.state='ready'
  else:
      self.state='not_ready'

An then we have process that does the actual processing without checking if in sockets exists or are connected. We should still only procduce data that is needed.

def process(self):
    radius=self.inputs.get('Radius')
    angle=self.inputs.get('Degrees')
    vert_nr=self.inputs.get('Vert No')

    parameters=match_long_repeat([angle,vertnr,radius])
    if self.outputs['Vertices'].links:
        points = [self.make_verts(a,v,r) for a,v,r in zip(*parameters)]
        SvSetSocketAnyType(self, 'Vertices',points)

etc.

Note the name process could be execute or something else.

States

• Not ready, the node has not sufficient inputs or outputs to be processed.
• Ready, the node can produce data.
• Fail, if it encounters some error or throws an error

More?

If any node in connected graph is marked not ready the node graph is not processed.
If any node in a connected graph fails the processing of that node graph is halted, the error node is marked.

Empty input

The question is what should happen on empty input.

Implementation stages:

Since this is big change it would be hard to do it in one step.

1. Fix the update system to handle nodes that has both process and update but treat them as one. First call node.update() then if it exists node.process(). It would work like today.
2. Split all nodes into two parts. Most well written nodes be very simple to change.
3. Change the update system.

Limitations

Data cannot drive number of outputs without user interaction (clicking a button for example). There is only one node today that does this, List Separate and that could be done in different way.

[ly29]

I did some further research and the issue is actually worse than I thought re:editing. Already today we could gain a lot by renaming all update(self) functions so that blender don't call them but only the sverchok update system. Adding a cylinder node triggers the update function 18 times...

Code:

 def init(self, context):        
        a=self.inputs.new('StringsSocket', "RadTop", "RadTop")
        print("Added socket:",a.name)

etc

   def update(self):
        print("Cylinder total sockets",len(self.inputs)+len(self.outputs))

Output:

Cylinder total sockets 1
Cylinder total sockets 1
Added socket: RadTop
Cylinder total sockets 2
Cylinder total sockets 2
Added socket: RadBot
Cylinder total sockets 3
Cylinder total sockets 3
Added socket: Vertices
Cylinder total sockets 4
Cylinder total sockets 4
Added socket: Height
Cylinder total sockets 5
Cylinder total sockets 5
Added socket: Subdivisions
Cylinder total sockets 6
Cylinder total sockets 6
Added socket: Vertices
Cylinder total sockets 7
Cylinder total sockets 7
Added socket: Edges
Cylinder total sockets 8
Cylinder total sockets 8
Added socket: Polygons
Cylinder init done
Cylinder total sockets 8
Cylinder total sockets 8

[ly29]

Also the blender update system updates nodes in the order given by keys()
If we add an prints(self.name) in update.

D.node_groups['NodeTree'].nodes.keys()
['Viewer Draw', 'Circle', 'List Range.001', 'List Range.002', 'List Range', 'Cylinder.001']

and then

Viewer Draw
Circle
List Range.001
List Range.002
List Range
Cylinder.001

Given all of this it sometimes feels like miracle that sverchok is working as well as it is...

[zeffii]

that's great, so a lot less processing seems achievable.. to the point
where the amount of processing expected is closer to what gets done.

[ly29]

Yeah, note that this is only for editor changes but if you click the update button or modify a slider, it works as it should. I am sure we all dealt with side effect of this while bug hunting...

Time for some sed and testing...

[ly29]

Not so happy about renaming all update functions even if seems to be working fine. It feels dangerous somehow.
Any suggestions for a new name for update? used sv_update when testing.

[zeffii]

sv_update is clear enough I think. So we dispatch to sv_update() from update() if the state of the node is good for meaningful processing..?

[ly29]

No we have to rename all update(self): to something else because update is called by blender.

The name change is a simple fix to decrease amount of updates on editor changes to 50% since blender ui code call the function.
It is also simple to add a bool in the node group update function to cancel recursive calls to the update system. Has some issues with crashes during update.
Then we get one update event instead of 18 for adding a Cylinder node.

[zeffii]

Does that mean that sverchok would be responsible for all updates? ..like calling sv_update() instead of blender calling update(). I think I get it. It is possible to filter out obviously pointless calls to sv_update() then ?

[zeffii]

so the stuff we used to do inupdate() would in the future be done inside sv_update() which should be called a whole lot less often.

[ly29]

If we change the names of all node level update() then we can implement a recursion check in the tree level update function.

[ly29]

I did something like this to test but it fails sometimes.

    sv_isUpdating = BoolProperty( default=False )

    def update(self):
        '''
        Rebuild and update the Sverchok node tree, used at editor changes
        '''
        if self.sv_isUpdating:
            print("Recursion protection!")
            return
        self.sv_isUpdating = True    
        try:
            makeTreeUpdate2(tree = self)
            speedUpdate(tree = self)
        except Exception as e:
            print(e)
            self.sv_isUpdating = False
            return
        self.sv_isUpdating = False

[zeffii]

is this the same?:

    sv_isUpdating = BoolProperty( default=False )

    def update(self):
        '''
        Rebuild and update the Sverchok node tree, used at editor changes
        '''
        if self.sv_isUpdating:
            print("Recursion protection!")
            return
        self.sv_isUpdating = True    
        try:
            makeTreeUpdate2(tree = self)
            speedUpdate(tree = self)
        except Exception as e:
            print(e)
        finally:
            self.sv_isUpdating = False

[ly29]

It should be, I try to stay clear of try except in special cases like this.
If you want to test just to change all update to sv_update except the one in node_s.py
Also on line 831 in util.py do_update

nods[nod_name].update()

Needs to be

nods[nod_name].sv_update()

But please don't look to much around there... there is room for cleaning up.

[zeffii]

I'm also not a fan of using try/except , but speed wise it's OK on code that isn't on a tight loop.

[zeffii]

I might have a read anyway to see if I can understand the mechanics.

[ly29]

But for some reason it doesn't work like this. Hmm.

Well we cut away 50% of the updates with rename alone. A step forward.

[ly29]

Can't really get recursion check working...
Changing the name of the update function seems to be working fine and it seems to as the right solution. However I will be busy for a while I want be able to clean up things that might break because of it.

However the startup issue is that update on node group level is also called. Which we can do an ugly fail on and then have it working via a post load handler. For now this seems to be working well. Will do further testing.

[zeffii]

what about simple UI drags of nodes across the canvas, I see blender really takes a hit on my machine when moving nodes.

[ly29]

I see that sometimes but havn't found out exactly what causes. But our updates are not called while moving nodes. Some draw function that does something really stupid? or blender? Don't know. Dragging a noodle triggers an update on release. Connecting with F gives two events, one is cancelled because the node group has an invalid link.
Adding a node or actually a socket can create series of events.

[nortikin]

We can ask Lukas Tone maybe

[ly29]

Yes I should try to collect a set of questions and requests.

[ly29]

Okay I think the way forward here is that I do limited with a couple of test to see if actually works out as well as I think it will and work out issues.
The sv_update rename we could do directly.

[ly29]

Now it is possible to dump update list from a tree using this:

>>> D.node_groups['NodeTree'].get_update_lists()
(['List Input', 'List Range Int.002', 'Cylinder', 'Viewer Draw.002', 'Plane', 'Viewer Draw.001', 'Circle', 'MaskList', 'Vectors Move', 'List Join', 'Viewer Draw'], {'Cylinder': ['Cylinder', 'Viewer Draw.002'], 'Circle': ['Circle', 'MaskList', 'Vectors Move', 'List Join', 'Viewer Draw'], 'Plane': ['Plane', 'Viewer Draw.001']})

For debug reasons. (main_list,{node:partial-list})

[ly29]

Nodegroups

I have been thinking about how to make reusable components of nodes. Already today it is kind of possible using wifi nodes, however it they can't be reused. It would be possible to do node groups with only a limited amount of works, however they wouldn't behave as nicely as blender node groups does with editing and so on. They would also have to live on either the same layout or a special layout only for node group since UI and processing is the same.
Which brings me to the next issue.

Total separation of processing and ui

To be able to have node groups without them being visible somewhere we need to totally decouple ui and processing, something that needs to be done sooner or later anyway. Does it make sense to do this as two steps or only make a large step? Even the first step would be a step forward and solve many issues. It would also make it easier to get to the next step.

Note This is important but some work to get started. But I will try to push forward with this.

[zeffii]

the hidden complexities of this beast scare me a little 👍

[ly29]

Yeah it does. I no longer think that a gradual transition is possible but resolved some other issues... Slowly.

[ly29]

I think we can do something usable without going C/C++ and learn a lot about how we want it to work. There will be limits of how much complex data we can process in real time, some algorithms are very heavy, like spatial search with kdtree, voronoi and others. From there we can make a proposal and try to redo it in C/C++ as a python module. Or somebody else learns from this project. But going this route still has some limits getting data in and out of blender I think, but much more complicated things could be possible. I found this interesting.
http://blenderthings.blogspot.nl/2014/04/native-python-extensions-for-blender.html
Of course going C/C++ will slow down development a bit...
PyNodes update function that we hack onto is just meant for a node to react and connect and disconnect and similar events.
If we used numpy completely, would it be worth the effort?

[zeffii]

Compiling a module may happen eventually, but multi-core access is a lot more attractive to investigate first (i think). And Numpy is perfect for all this numerical stuff, but I think you mentioned this before -- all data passed around would need to be in np.array([]) form to avoid the overhead of creating it each time. np.array silently optimizes it's containers if it realizes it's holding homogeneous data: https://scipy-lectures.github.io/intro/numpy/array_object.html

maybe this is off topic.

[ly29]

It is slightly off topic, but it does needs to be done also. But too separate ui and processing is very important step that will make later work easier.

[ly29]

When we do this split we can stop expensive updates on every editor change. Further down the road we keep track of changes in nodes and update only changed nodes, like works like now if you change a slider in a node, only the nodes affected will have to change.

[zeffii]

let me ask some more questions then.

• what changes (if any) would need to be made to the code of the current nodes? What would that look like for instance for List Range Int node?

[ly29]

Current:

    def update(self):
        inputs = self.inputs
        outputs = self.outputs

        # outputs, end early.
        if not 'Range' in outputs or not outputs['Range'].links:
            return

        param=[inputs[i].sv_get()[0] for i in range(3)]
        f=self.func_dict[self.mode]
        out = [f(*args) for args in zip(*match_long_repeat(param))]
        self.outputs['Range'].sv_set(out)

note the last self there can and should be removed.
New

def update(self):
        # last socket created, now we are ready!
        if 'Range' in outputs:
            self.state='ready'

def process(self):
        inputs = self.inputs
        outputs = self.outputs
        if not outputs['Range'].links:
            return
        param=[inputs[i].sv_get()[0] for i in range(3)]
        f=self.func_dict[self.mode]
        out = [f(*args) for args in zip(*match_long_repeat(param))]
        outputs['Range'].sv_set(out)

The main difference is that process will only be called by the sverchok update system and blender can call all update functions, it finishes with the one on node group level. From there sverchok will call process on nodes graphs where all nodes are ready.

Perhaps we can even say the it has to terminate in an output node for it be processed. For example view draw, text out etc. In which case the check that 'Range' links could be done in update also.

Actually it feels logical to check for 'Range' there. No?

[zeffii]

Yeah the availability of terminating nodes would be the last check before calling process

[zeffii]

I think you are saying that even the check for

        if not outputs['Range'].links:
            return

might be better suited in update ?

in that case perhaps the signature of process could be process(self, inputs, outputs) if that makes sense

[ly29]

Yes, that we should check that it makes sense to call process. I am not sure, a lot of these issues needs to be resolved while implementing it I guess.

[zeffii]

this may be tangential, but it's not permitted to update .self while inside draw_buttons

AttributeError: Writing to ID classes in this context is not allowed:

i do recall there are overrides for this

http://blender.45788.x6.nabble.com/Python-API-change-breaks-Rigify-td104240.html

This is possible to do now, if you define an RNA property you can give
it an update callback to run.

[ly29]

process(self) vs proces(self, inputs, outputs)
Passing the input and outputs as parameters doesn't make sense since they are available in the class.
List Range Int is a simple example since it can always produce output and only has one output, more complex cases have to check which outputs are linked to know what it needs to produce. I will start doing this soon, at lot things will clear once it is implemented. Enough thinking, time to start doing.

[ly29]

Refactoring the update system a bit in preparation. To separate unconnected node groups on the same layout to allow error handling to be for each sub group of nodes.

Updated  NodeTree.001
separate:0.0004 
[make_update_lists] 0.0033

Also a nice performance boost on the same layout:

make_update_lists 
new: 0.0034 
old: 0.0177

5 times quicker, basically by having less . and doing less ng.nodes[name].inputs. The things that you can speed up by stop being stupid. Some more testing... Separate is the same code as in SeparateMesh. Reuse!

[ly29]

Note that this rethinking happened because @zeffii posted the python performance tips and that I noted that separate this is very similar to building the list only took a fraction of the time

Some further testing and looking at performance problems we are now 100 times faster for a larger test layout (173 nodes)

make_update_list new 0.0052 old 0.5412

The point of the new code was make independent groups in the layout separated, this is right now "only" 56 times faster than building a complete layout with old make_update_list
Which allows for more fine grained control of error handling etc. This could be further fine tuned but I think I will stop now.

make update split 0.0035 [list comp] 0.0061

So how is this speedup possible? By doing less inspection and caring less about absolute order of nodes, only that the nodes they depend on are updated before them.
Before there was some attempts to sort the list better to make sure that we could jump a head in update list from updateNode, but all this clever work cost some time, and the jump in list for a partial update never really made sense. Shortly after I did that code, I changed and built partial update list as need, which makes sense for all but the most trivial cases, which are quick enough anyway...
Now I have look for edge cases with this new system before committing it. But we think have reached a performance level which makes the building of update lists a small problem for now.

[nortikin]

good explanation. Good speedup for update in python is limited anyway, but cython or something may help in far future.

[zeffii]

This UI and processing separation could allow for the node window to be an optional external application, even a browser window. This might lead to the node view being rewritten in javascript / coffeescript and it would communicate via sockets with Blender to change the states of nodes. Think about the level of customization that could be implemented into the external nodeview.

maybe in the distant future

[zeffii]

Would this mean for the code of the nodes? All processing functions declared outside of the class? or entirely different .py files which the node imports.

[ly29]

It could be done in many ways. We should just decide. A separate file would be good because it would force separation. I imagine either a compile function that returns either a class or a callable, or a class that can parse the node and extract the needed info.
It is also a good time to clean up nodes, mark side effects nodes (input/output) animatable ones etc.

[ly29]

A lot of into in this thread isn't up to date with my current thinking about the issue, I think I will have time to tackle this next week and will probably start an new issue about it then.

[zeffii]

Maybe do a demonstration node, something simple, and also a more complicated one that really pushes the node interface. Okay, in your own good time.

[nortikin]

test node as usual. If your look at UI/workingdef separation changed, what changed?

[nortikin]

and my opinion, if no viewer draw/bmeshviewer etc in chain of update, than no update should occure with this nodes... imho. Or even if viewer node not active

[ly29]

@nortikin
Exactly, also if we have a switch node it should we should only process the needed node tree etc.

Reprocess as little as possible, store as little data in the socket cache as possible but still allow small changes quickly.

The big change from what outlined above is to separate the execution completely from the nodes which would allow a simple form of real node groups to exist, the editing wouldn't be as nice as blender but it could be fully functional.

[ly29]

This is more and more becoming a limitation, therefore closing the issue might seem like the wrong thing to do but it is and old issue I think it better to start over then pretend that is is moving forward right now. I am not sure how to do this the correct way but will outline my ideas and concerns today in a new issue. But there is lot noise here so closing this one.