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

Fix port recommendations #170

Merged
merged 9 commits into from
Mar 16, 2023
Merged

Fix port recommendations #170

merged 9 commits into from
Mar 16, 2023

Conversation

liamhuber
Copy link
Member

Node recommendations were easy because we just needed to check if a node could support the demands. Port recommendations are harder because we need to check if a node instance does support the demands, i.e. we need to check if the current workflow graph is a subset of all possible ontological workflow trees.

With the more complex surface energy example, I discovered that some of the transitive demands were not being passed in the old, simpler port recommendation. So here we really do the graph-to-graph comparison described above. It's working great, and you can even invalidate a bunch of upstream stuff on-the-fly by re-wiring it to the wrong sort of downstream input. However the already un-performant task of selecting a new node is even worse; from something like an 0.7s delay to something like a 1.5s delay. I want feature and functionality before efficiency, so let's keep this and make it more performant in a follow-up.

@github-actions
Copy link

Binder 👈 Launch a binder notebook on branch pyiron/ironflow/fix_port_recommendation

Remove redundant variable
It still has the chance to make multiple tree construction calls, but this still seems unavoidable to me right now
By using iterators instead of list comprehensions in `any` and `all` calls
@liamhuber liamhuber marked this pull request as ready for review March 16, 2023 19:37
@liamhuber liamhuber merged commit 75df989 into surface_energy Mar 16, 2023
@liamhuber liamhuber deleted the fix_port_recommendation branch March 16, 2023 19:37
@liamhuber liamhuber mentioned this pull request Apr 5, 2023
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.

1 participant