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

tree_at silently skips __check_init__ #872

Open
jeertmans opened this issue Oct 8, 2024 · 2 comments · May be fixed by #874
Open

tree_at silently skips __check_init__ #872

jeertmans opened this issue Oct 8, 2024 · 2 comments · May be fixed by #874
Labels
question User queries

Comments

@jeertmans
Copy link

Hi!

I guess this is probably a "feature" of tree_at, but I recently discovered that it does not run __check_init__, which means it can bypass checks, leading to malformed data.

I could not find any reference to this in the documentation, so (1) is it possible to still have __check_init__ run after some tree_at surgery, and (2) would it be possible to document that in the docstring of the tree_at function?

As always, thanks for your great tool :-)

@patrick-kidger
Copy link
Owner

This is intentional! tree_at skips this in the same way that it skips __init__ itself.

As for why, it's common for these functions to be used to assert that certain invariants hold (e.g. assert self.some_value > 0), but it's also common to use tree_at in cases where the tree is being used polymorphic with respect to leaf type (e.g. int_tree = tree_map(lambda _: 0, some_tree); int_tree = tree_at(lambda t: t.s.u.v, int_tree, 1) for constructing the argument for vmap(..., in_axes=...))

So if we ran __check_init__ then we wouldn't be able to express this!

I'd be happy to take a PR clarifying the documentation on this.
(And if you want to run __check_init__ then I recommend creating your own tree_at wrapper that does this. :) )

@patrick-kidger patrick-kidger added the question User queries label Oct 8, 2024
jeertmans added a commit to jeertmans/equinox that referenced this issue Oct 8, 2024
Hi @patrick-kidger!

Here is a first proposal. I am not entirely convinced by my wording, but let me know what you think of it.

Closes patrick-kidger#872
@jeertmans jeertmans linked a pull request Oct 8, 2024 that will close this issue
@jeertmans
Copy link
Author

Thanks for your reply @patrick-kidger! I had doubts this was intentional, just wanted to have clarification and also think this deserves to be documented :-)

I did a small PR to this end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question User queries
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants