-
-
Notifications
You must be signed in to change notification settings - Fork 154
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
Strange error with multiple tuples as attributes and equinox.tree_at
?
#715
Comments
Not 100% sure on the source, but one fix is to force it to recognize tuples as endpoints, via import equinox as eqx
class M(eqx.Module):
a: tuple
b: tuple
def __init__(self):
self.a = ()
self.b = (1,)
eqx.tree_at(lambda m: m.a, M(), (), is_leaf=lambda x: x == ()) |
Interesting! Looks like Python actually uses the exact same object for tuple literals: () is () # True
(1,) is (1,) # True So the logic inside Now specifically leaves frequently fail this (e.g. And until this point, there haven't been any examples of composite nodes which fail the If anyone feels like tackling this in the near future then I'd be happy to take a pull request. |
Is this something that needs a fix, it seems like its working as it should? This exists because an empty tuple is a singleton in CPython, so only 1 ever exists in memory (so all () have the same id) (https://stackoverflow.com/questions/14135542/how-is-tuple-implemented-in-cpython/). If you have something like class M(eqx.Module):
a: tuple
b: tuple
def __init__(self):
self.a = (1,)
self.b = (1,) this works because the id's are different. In my mind, equinox is behaving as it should, the where function is underspecified since it points to two objects in memory and its hard to think of a workaround that works for making tuples which are identical in memory that doesn't break other things that are equal in memory (outside of a hardcoded |
So I think of I think on that basis this is a minor bug. Though as you say, this is a very very edge case with a loud failure and an easy work-around, so it's not one I'm too worried by overall! |
Hmmm I see. This is a fix, by making a wrapper object basically for the tuple singleton: #717. This is definitely the craziest edge case to write this much code for lol |
Hello!
I've come across an error I'm not quite sure what to do about.
MWE:
Running the above gives me the following error:
However, if I use lists, or a tuple and a list
or just a single tuple
there is no error.
The text was updated successfully, but these errors were encountered: