You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
FunctionGraph.change_input changes an Apply node's inputs in-place, but we could just as easily clone and replace the affected Apply node(s).
FYI: FunctionGraph.change_input is what drives FunctionGraph.replace, and the latter is what—in turn—drives all the graph rewriting that occurs with GlobalOptimizer, LocalOptimizer, etc.
This approach would prevent the need to clone graphs before creating a FunctionGraph, by—instead—cloning "on-demand" and only when and where it's actually required. It would also be a convenient way to preserve the identity of unrelated nodes and reduce the need for a lot of clone-to-original-variable mapping logic.
I started an implementation here (and a test here), but that simple approach won't really work.
Also, we would need to provide a new callback for this type of update, and, if we make this approach the default, we would need to update the Features that use the old "on_change_input" callbacks so that they use this new one.
The text was updated successfully, but these errors were encountered:
This would help with one of the problems in #431, since this approach to graph mutation doesn't avoid calling Apply.__init__.
brandonwillard
changed the title
Provide a non-in-place version of FunctionGraph.change_input
Implement a non-in-place version of FunctionGraph.change_inputMay 24, 2021
FunctionGraph.change_input
changes anApply
node's inputs in-place, but we could just as easily clone and replace the affectedApply
node(s).FYI:
FunctionGraph.change_input
is what drivesFunctionGraph.replace
, and the latter is what—in turn—drives all the graph rewriting that occurs withGlobalOptimizer
,LocalOptimizer
, etc.This approach would prevent the need to clone graphs before creating a
FunctionGraph
, by—instead—cloning "on-demand" and only when and where it's actually required. It would also be a convenient way to preserve the identity of unrelated nodes and reduce the need for a lot of clone-to-original-variable mapping logic.I started an implementation here (and a test here), but that simple approach won't really work.
Also, we would need to provide a new callback for this type of update, and, if we make this approach the default, we would need to update the
Feature
s that use the old"on_change_input"
callbacks so that they use this new one.The text was updated successfully, but these errors were encountered: