pyiron/ironflow

Port recommendations are not performant

Closed this issue · 3 comments

Selecting ports was already slow to refresh the GUI (maybe 0.5-1s lag), but since #170 with the ontologically powered graph-comparison for port recommendations, it is roughly twice as slow.

Building the node recommendations (gui.build_recommendations(surface_en.inputs.ports.bulk_energy)) takes ~37ms, so that's fine. On selecting a node, the recommended tab also gets updated promptly in the gui.

Getting the ontologically compatible ports inside the existing workflow is super slow; gui.workflows.flow_canvas._get_port_widgets_ontologically_compatible_with(surface_en.inputs.ports.bulk_energy) takes 3.25s using %%timeit on a basic surface energy example. (I expect the lag to scale at least linearly with graph size, and don't fundamentally object to that as long as what we're scaling is a small enough thing to start with.) So this is really slow entirely apart from the GUI!

The only approach I've been able to think of to reliably get the desired behaviour is a graph-to-graph comparison to make sure the actual ironflow workflow graph is a subset of ontologically possible graphs. But I was just busy getting that working in #170, so let's go back and see if it can be made more performant.

Test scenario:

  • Using a simple 8-node graph for surface energy calculations, modifying code from #170 as needed.
  • Using the surface energy input as a trial port
  • %%timeit to test various chunks of the raw commands

Steps and results:

  • Select a port
    • With all recommendations turned off, surface_energy_port_widget.select(); surface_energy_port_widget.deselect() takes 734 ns (actually doing it in the gui is slower than this, but that is from updating the visuals)
    • With all recommendations turned on, takes 4.37s -- so we have basically the entire 4.4s to resolve
  • Build node recommendations
    • Linear in the number of registered nodes, calls get_downstream_requirements once, calls otype.get_sources once
    • gui.build_recommendations(surface_energy_port) takes 50.2 ms -- really not a problem right now
  • Highlight compatible ports
    • Linear in the number of ports currently in the graph ($n$), calls otype.get_source_tree and get_downstream_requirements $n$ times -- doesn't need to! I think we can get away with 1x on the selected port
    • Does a recursive graph comparison where the scaling is not immediately obvious, but I can probably work out the worst-case if needed
    • gui.workflows.flow_canvas.highlight_compatible_ports(surface_energy_port_widget) takes 4.1s, so we're missing ~0.2s, but rerunning I get noise on this order so I don't think that's a sign of big trouble

Clearly the port highlighting is the big culprit, and I can already see one algorithmic improvement there.
Let's drill down.

  • gui.workflows.flow_canvas._get_port_widgets_ontologically_compatible_with(surface_energy_port) takes 3.5s, so here is at least most of our trouble -- the actual highlighting (scales with the number of ports found, typically few) afterwards seems minimal
  • Building the tree input_tree =surface_energy_port.otype.get_source_tree(additional_requirements=surface_energy_port.get_downstream_requirements()) only takes 0.25s, but this is currently scaled by the number of I/O ports in the graph that match our O/I selected port. Right now that's 48/62 ports, so this step is actually taking something like 14*0.25 = 3.5s instead of 0.25s. Looks like we found our culprit.
  • For completeness, let's test the worst-case available of the tree comparison -- the calc min output from the correct branch, since all other paths will fail much earlier but this one will pass -- surface_energy_port._output_graph_is_represented_in_workflow_tree(surface_min_en_port, input_tree) is 65microseconds, so I think we're ok

So it looks like we just need to go back and make sure that the tree is only constructed once, instead of once per IO-pair. I think this should be possible.

Ok, only getting the tree once when selecting an input port drops the python time for gui.workflows.flow_canvas.highlight_compatible_ports(surface_energy_port_widget) to 0.24s. There is still about 1s of lag in the gui, but I think this has to do with the canvas redraw.

Still trying to figure out how to get around all the tree calls when selecting an output port.

Couldn't figure out how to avoid multiple tree construction for output ports, but did filtering to build them for as few input ports as possible.

The select/deselect test we started with now takes 290ms. Much more than the 734ns without any ontological type checking, but getting close to bearable and a serious improvement over the 4.4s before these corrections.
The same check on an output port (in this case the bulk structure output structure) takes 444ms -- about twice as long; this is exactly as expected because the workflow has two plausible inputs corresponding to this port, and they each independently make the ~250ms step of building their tree of workflows.

Next I'll go look at the ontology code to see if the tree construction itself can be sped up; in principle we also don't need the whole tree, but could do a branch-by-branch comparison... but let's save that for if we get desperate.
Because we feed the downstream requirements based on the actual working graph -- which are dynamically dependent on graph connections! -- we unfortunately can't just pre-compute each tree once.

Along with upstream performance improvements in pyiron_ontology #12, selecting and deselecting ports (via code, there is prexisting lag on the GUI rendering side) is down from 4.4s to 52ms. Still slow enough that it doesn't feel snappy in the GUI, but definitely usable now.