-
Notifications
You must be signed in to change notification settings - Fork 129
Implement OpFromGraph
__eq__
and __hash__
#1114
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
Comments
OpFromGraph.__eq__
OpFromGraph
__eq__
and __hash__
Do we want to support Alpha equivalence where the following two are considered equal? x = pt.scalar("x")
out1 = OpFromGraph([x], [x + 1])(x)
y = pt.scalar("y")
out2 = OpFromGraph([y], [y + 1])(y)
assert equal_computations([out1], [out2]) |
Those Ops are equivalent, but not the nodes, since one takes x, and the other takes y. So x = pt.scalar("x")
out1 = OpFromGraph([x], [x + 1])(x)
y = pt.scalar("y")
out2 = OpFromGraph([y], [y + 1])(x)
assert equal_computations([out1], [out2]) The dummy variables used to define the inner graph are irrelevant.
|
Oh right, my mistake!
…On Fri, 3 Jan 2025, 19:08 Ricardo Vieira, ***@***.***> wrote:
Do we want to support Alpha equivalence
<https://en.wikipedia.org/wiki/Lambda_calculus#Alpha_equivalence> where
the following two are considered equal?
x = pt.scalar("x")out1 = OpFromGraph([x], [x + 1])(x)y = pt.scalar("y")out2 = OpFromGraph([y], [y + 1])(y)
assert equal_computations([out1], [out2])
Those Ops are equivalent, but not the nodes, since one takes x, and the
other takes y. So out2.owner.op == out1.owner.op should be True but
equal_computations not. However the following should be:
x = pt.scalar("x")out1 = OpFromGraph([x], [x + 1])(x)y = pt.scalar("y")out2 = OpFromGraph([y], [y + 1])(x)
assert equal_computations([out1], [out2])
The dummy variables used to define the inner graph are irrelevant.
—
Reply to this email directly, view it on GitHub
<#1114 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAACCUMBRSBBXEOAJTDUANT2I3GY3AVCNFSM6AAAAABTI3V3KWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNRZGYZDKNBTGY>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Description
This allows merging duplicated nodes as well as comparing graph equality.
It should pass the assert. It fails because
out1.owner.op == out2.owner.op
evaluates to False. We can probably do something very similar toScan
:pytensor/pytensor/scan/op.py
Lines 1254 to 1320 in 4b41e09
The text was updated successfully, but these errors were encountered: