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

New update system #4164

Merged
merged 77 commits into from
Jul 12, 2021
Merged

New update system #4164

merged 77 commits into from
Jul 12, 2021

Conversation

Durman
Copy link
Collaborator

@Durman Durman commented Jun 12, 2021

Addressed problem description

This is third attempt - #3179 #2393
I fill this will be last one.))

Closes #3058
Closes #3795

New features

  • It will be possible to cancel node tree updating
    But not always. It can cancel only between nodes but not when a node is working. It is more flexible with node groups. It will possible to evaluate them one by one internal node so in this way work of a group node can be canceled. Evaluation can be canceled by pressing escape but also it is possible by changing a node value before a tree is fully evaluated. In this case the change of the property will cancel current evaluation and start new one. Also canceling is possible during animation (the same way as with property changed) but probably it is not always expected and there should be an option.
    cansel updating by draging a property

  • If a node tree has more then one disconnected node graph they will be evaluated independently.
    It means that an error in one subgraph does not prevent updating of another. Also error messages will be more persistent and wont disappear until the cause to be resolved. But there is also another side effect. Nodes will be never reevaluated if there is a node with an error before. In current update system in some cases it's possible and I don't know whether I should change the logic here somehow.
    new error handler

  • Supporting muted links
    muting links

  • show progress in the header of node tree editor

2021-06-28_14-17-57

  • support tree annotations inside node groups
    2021-06-17_10-59-42

  • cumulative update time

cum time

  • Changes in panels

2021-06-28_14-19-34

Supporting old features

  • wifi nodes

  • loop nodes. Actually I would prefer not to touch them but I know that they reuse some code from the update system module and it may appear that they expect the module to be in some state to work correctly.
    I've checked loop nodes seems there is no need in any changes.

  • print timings in a node editor. Just show time of a last update. Can be hidden in the panel of tree properties.

  • add profiling

Unsupported old features

  • Evaluation order according nodes position. The approach looks doubtful to me. If an order has metter it should be set explicitly with links I think. And would prefer not to overcomplicate the update system. But if some one interested it could be implemented ofcause. In this case new sorting method should be added to the utils/tree_walk.py module. And the event should get extra option I guess.

  • Show error stack in a node tree editor. It looks like useless for users and is more needed for devs. I expect last ones use system console for that since there is other useful information there.

  • Showing time graph. I would not mind to connect it to new update system but it's not connectable. It uses the update system module directly. Proper place to connect it would be in tree.update_ui method. This is new method called by new update system when the job is done. Updating time can be get from tree.handler.get_update_time(). The only problem will be with evaluation order because it is not kept in statistic results currently but could be added.

  • Heat map. It is tightly bound to current update system and can't be reused. Showing update time in a tree editor partly covers this functionality. In the future I have plans of improving debugging tools as well.

Plans for next PRs

After this PR the throttle decorator and context manager will be deprecated. Evaluation of a tree will be in another Python session and it wont be effected by extra updates of a tree. It will be possible to manipulate with sockets more easily for the same reason. When a tree is evaluated the sockets will already be with the correct is_linked state.

Add more functionality for debugging. I'm thinking of adding list widget to the tool panel. It can show list of all nodes in a tree and their updating time. List widget has sorting and filtering UI which will be quite useful to search, for example, slow nodes. A list item is selectable so it will be possible to give extra information about selected node. Probably it could have an operator - "go to selected node".

Add vectorization loop nodes which would include last ideas about how the vectorization should work. Also it should be possible to cancel their work. It should be possible to debug each iteration. Implementing them could be simpler then past the vectorization into all existing nodes.

Think how the work of the socket_data module could be delegated to update system.

Preflight checklist

  • Code changes complete.
  • Code documentation complete.
  • Documentation for users complete (or not required, if user never sees these changes).
  • Manual testing done.
  • Ready for merge.

Durman added 7 commits June 12, 2021 14:30
…or. It has clear format of properties and is easier to use.
…al it similar to code of the node group handler but simpler. All nodes has real ID which should not ever be changed. There is no need in global handler because it is already known which tree was changed and its changes can effect only inner nodes and subtrees.
…date calls of old update system. Also add `update_ui` method to the tree and the node classes. They should be used to reevaluate view (error and timing prints, node colors) of the nodes in the tree.
… It's now used by several modules and this place looks more appropriate.
… Update ui should get into previous commit actually
@Durman Durman marked this pull request as draft June 13, 2021 02:36
Durman added 22 commits June 13, 2021 13:44
…lf correctly. Next event saw that there is a task and try to cancel it throwing CancelError into the handler but it returns some unexpected error and finish_task method was unreachable
…no need in throttling updates and calling update tree after all). Light refactoring of the module.
… (actually there should be no extra steps, trees will update properly upon Blender call to trees to update). Add some cleanings before opening new file.
… and the draw function every time checks a node position than drawing text of all nodes takes O(n**2)
…calls to node update methods. Main tree handler expects event of the tree changing instead of nodes changing
…on about nodes execution. Simplify tree handler, it delegates part of its functionality to new node handlers (also generators). Add node handlers. They execute nodes, keep their appropriate update state, return success of nodes execution. Node structure now has separate update state for input, inner state and output
…f functionality to node updaters. Add node updater which specific group tree update system. Update system of main tree and group tree can't share enough code between each other because group tree update system is overcomplicated by logic of mutable node IDs. At the present we can't avoid using this hack.
Durman added 5 commits June 28, 2021 13:11
…ly inside a group tree for now. Initial statistic of update time now is kept in seconds (float).
…aluation the garbage collector was slowing down the process. Update time of some nodes can change significantly without changing its input data from one tree evaluation to another (this was also noticed in old update system). With garbage collector disabled such time jumps seems disappeared. Evaluation speed of tree was increased in some cases in 1.5 times. Also I did not notice any problems with memory usage.
# Conflicts:
#	data_structure.py
#	ui/sv_3d_panel.py
@Durman
Copy link
Collaborator Author

Durman commented Jun 28, 2021

The code is more or less ready. I don't have plans of adding anything but bug fixes now. If someone want to be sure not to have problems with new update system it's time to test it.

@Durman Durman marked this pull request as ready for review June 28, 2021 10:38
@vicdoval
Copy link
Collaborator

@portnov sverchok-extra fails with this PR due:
from sverchok.core.update_system import process_from_node in triangular_mesh.py process_from_node can't be imported

Another little issue: I have a node-tree in my default file, this node-tree is not evaluated until I interact with it (click on toggle, make a link, even if it is just start the link and cancel it). This does not happen if I open blender by clicking in a file, only with the default file. With the actual update system does not happen.

@portnov
Copy link
Collaborator

portnov commented Jun 29, 2021

@Durman is there a method in new update system to be used instead of process_from_node?
The problem is: some node(s) is too slow to process data "online", when you change anything in the tree. So it has explicit button "process!" to trigger processing.

@Durman
Copy link
Collaborator Author

Durman commented Jun 29, 2021

@portnov node.process_node is it not what you want?
@vicdoval loading startup files fixed

@portnov
Copy link
Collaborator

portnov commented Jun 29, 2021

will try process_node.

@vicdoval
Copy link
Collaborator

@portnov you could also check how I solved that problem in the "Evolver" node, in which the process function is only in charge of passing the data from a dictionary to the next node

@vicdoval
Copy link
Collaborator

@Durman great, now loads perfectly.
Some ideas:
Should node timings be activate by default? maybe is to technical and at the beginning everything is just 0ms

Does it really improve performance that the timings position only changes on nodetree update? The actual solution (moving when you move nodes) looks much more responsive.

@Durman
Copy link
Collaborator Author

Durman commented Jun 29, 2021

Yes, probably the timings can be disabled by default but we can leave it as it is for a while so more people could be aware of this feature.

Problem with tracking position of a node is that we can't put its reference into a draw function. So the function each redraw should search the node. According that all nodes have a massage to draw it it takes n2 time. It's not such a problem if only one node has massage or a node tree has a few nodes but with big trees (don't know probably handred nodes) the workflow becomes realy slow.
https://discord.com/channels/745273148018262158/806643543182802954/854253625806815242

Another solution could be to draw messages right in nodes. This is what is being done in GN.

@zeffii
Copy link
Collaborator

zeffii commented Jun 30, 2021

can dynamic_location be the default for the timings, even if it is inefficient. if a nodetree is so heavy that a user might have some benefit from static/less dynamic timer text location, then maybe leave that up to the user.

@Durman
Copy link
Collaborator Author

Durman commented Jul 6, 2021

It will be easy to add extra property to the Node timings panel which make timing prints stick with nodes.

@Durman
Copy link
Collaborator Author

Durman commented Jul 9, 2021

Are there any objections switching this to (1, 0, 0, 0) in this PR?

"version": (0, 6, 0, 0),

# Conflicts:
#	core/group_handlers.py
#	core/update_system.py
#	node_tree.py
#	ui/sv_temporal_viewers.py
#	utils/exception_drawing_with_bgl.py
@Durman Durman linked an issue Jul 12, 2021 that may be closed by this pull request
@Durman Durman merged commit 797d239 into master Jul 12, 2021
@Durman Durman deleted the new_update_system branch July 12, 2021 04:17
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.

Matrix Apply not working when Line + Line tree.sv_links.get_nodes(tree) Update system requirements
4 participants