-
-
Notifications
You must be signed in to change notification settings - Fork 153
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
Hash-cons Apply
s and Constant
s
#1165
base: main
Are you sure you want to change the base?
Hash-cons Apply
s and Constant
s
#1165
Conversation
Do we ever change Apply inputs inplace behind the scenes? |
Yes, in |
679da16
to
80e412e
Compare
f4a9e86
to
9843a4b
Compare
Any chance trying this could give us enough speedup without having to perform such fundamental changes? Edit: The suggested improvement in that issue was actually already implemented: aesara/aesara/graph/rewriting/basic.py Lines 772 to 774 in 7550668
|
We're not looking for just a speed-up here. We're trying to improve our graph object model, rewriting process, and graph equivalence checks in general so we can arrive at a place where #1082 —and other "meta" rewriting work—is feasible in Aesara. Regardless, incremental changes to For instance, with the changes in this PR, there's no need to run through the rewrite machinery to accomplish the same things as our current merging, and that machinery has a very real cost (e.g. cloning, numerous iterative object and type checks, etc.) As a result, there are considerably fewer limitations on when identity-based comparisons can be productively made between graph objects (e.g. within rewrites, graph-constructor/helper functions, etc.) Also, there are much fewer rewriting redundancies (e.g. no Currently, we have to wait for merge passes after every rewrite before we can reasonably ask when/if (sub)graphs are equivalent. This adds complexity and constraints to the construction and use of rewrite passes and the databases in which they reside. These changes also make changes like #996 considerably more straightforward, and they directly address things like #724. More importantly, if we ever want to cache and/or memoize rewrite steps, we'll need a more suitable object model—like the one here. |
Apply
s and Constant
sApply
s and Constant
s
4b81904
to
8b69775
Compare
e15e056
to
03d841f
Compare
dc20621
to
5b01bff
Compare
5b01bff
to
3e9665c
Compare
3e9665c
to
ad7259d
Compare
This was covered in some Gitter chats (e.g. some egraph conversations here) months back, but it should be here as well: with these changes in place, we can more efficiently—and easily—implement egraphs and Equality Saturation as a |
This is an experimental PR that attempts to use hash-consing to restrict equivalent
Apply
s andConstant
s to distinct representative instances. These changes should remove the need to perform "merging" as a distinct rewrite step, since it would now be performed by theApply
andConstant
"constructors".In other words,
Apply(op, [v0, v1], ...) is Apply(op, [v0, v1], ...)
andConstant(t1, x) is Constant(t1, x)
.In order for this notion of equivalence to work, we need immutable
Apply
s , which means that we need to stop updatingApply.inputs
in-place. To accommodate that requirement,FunctionGraph.change_node_input
can create newApply
instances with the updated inputs. Afterward, the oldApply
nodes need to be replaced by the new ones somehow.These changes overlap with the work in #666; however, we would like to avoid the need to clone and replace every
Apply
node that uses a replaced node's outputs as inputs (i.e. we don't want to clone and replace all the dependent sub-graphs).We should be able to avoid sub-graph cloning by moving the in-place updates to
Variable.owner
(i.e. re-assign the output variables to the newApply
s). The problem with a change like this is that ourFeature.on_change_input
callback semantics will also need to be changed, because—for instance—the objects passed to those methods will no longer have the same expected relationships with each other. For example, if we're no longer changingApply.inputs
in-place, a call toFeature.on_change_input
with a newly constructedApply
node as thenode
argument will be invalid forFeature
s that track thenode
arguments explicitly. In general,Feature
s will need to explicitly account for any new/different relationship between a new node and an old/replaced one.One approach to updating
Feature.on_change_input
involves the use of old and new node arguments. In this case, subscribers to theFeature.on_change_input
callbacks will be responsible for handling the node replacements, which—in many existing cases—is reasonable. Also, if we're updatingVariable.owner
in each of the output variables of an replacedApply
, listeners can start using "representative" output variables to track specific node locations in a graph. In other words, if one tracksout = node.outputs[0]
instead ofnode
,out.owner
will always reflect the applicableApply
node after all/any subsequent changes tonode.inputs
.We need to determine how distinct computations of an
Apply
node are represented in this scenario. Currently,z = x + y
andw = x + y
represent two distinct computations (e.g. expressed in implementation withz != w
beingTrue
). Under these changes,z is w
could beTrue
, and we would need to represent distinct computations of the same expressions in some other way (e.g. via the use of distinct outputVariable
s that point to the sameApply
).I think the only reason we currently need distinct, duplicate computations is that it's a necessary part of our memory/data-flow model—itself needed for things like in-placing. Instead, we can work in reverse to how we currently do by making all equivalent expressions represent the same computation by default (i.e. the changes in this PR), then we use special constructors/helper-functions and rewrite passes that introduce distinct computations as required (e.g. for in-placing). In other words, we go from everything being a distinct computation by default and then merging the equivalent ones, to everything being equivalent (that actually is equivalent) by default and then introducing duplicates when needed.
Again, distinct output
Variable
s could be used to represent distinct computations, since multipleVariable
s can have their.owner
values set to the sameApply
node. The primary difference with our current approach is that theApply
nodes associated with those distinctVariable
s will not be distinct.Our in-placing rewrites,
DestroyHandler
, andMerge[Optimizer|Feature]
work together—with the backtracking provided by theHistory
Feature
—to incrementally apply the equivalences between equivalent expressions. That approach is more oriented for other concerns or interests and doesn't really suit the needs of equational reasoning, term rewriting, and other areas of work that make use of these equivalences.