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

TF Nest alternative #1314

Closed
albertz opened this issue Apr 18, 2023 · 6 comments
Closed

TF Nest alternative #1314

albertz opened this issue Apr 18, 2023 · 6 comments

Comments

@albertz
Copy link
Member

albertz commented Apr 18, 2023

For some code, it is useful to have functions like what TF nest (from tensorflow.python.util import nest) provides, such as nest.map_structure, nest.flatten, nest.pack_sequence_as, and others.

So far, it was not really a problem to use just the TF nest, as we anyway have TF as dependency (although for RC, this was debatable: rwth-i6/returnn_common#27).

Now, as we support different backends again (#1120), we want to have a framework-independent solution.

So, what are the alternatives:

  • Create a simple implementation ourselves. It's not really so hard. But it's kind of reinventing the wheel.
  • Copy over some existing implementation (e.g. TF nest) and keep our own copy.
  • Use some other alternative package, i.e. add a new dependency.

(Similar discussion here: rwth-i6/returnn_common#27 (comment))

Some known implementations:

I did not really looked into them in detail whether they really exactly provide what we need (which is mostly exactly what TF nest provides).

@albertz
Copy link
Member Author

albertz commented Apr 18, 2023

I'm currently leaning towards those two options:

  • Copy over TF nest to RETURNN. Just because I'm familiar with it.
  • Use Tree from DeepMind. Seems well-maintained and exactly what we need (in contrast to the other packages which provide other stuff). But this would add a new dependency.

@curufinwe
Copy link
Collaborator

From these last 2 options I think I would prefer to add the dependency to tree. This will keep the RETURNN code small and more approachable to new developers.

@albertz
Copy link
Member Author

albertz commented Apr 19, 2023

Ok then it's decided: we use Tree from DeepMind.

@albertz
Copy link
Member Author

albertz commented May 10, 2023

@albertz
Copy link
Member Author

albertz commented May 10, 2023

Note that this is currently only used (and needed) for the RF (returnn.frontend).

Edit Ok, the RF is also imported when the engine is selected in RETURNN, to initialize and set a reasonable RF backend. This is done always at startup. I'm not sure if we want to avoid this in any way. It would probably get ugly. But so it means everyone really needs to install dm-tree now. But maybe it's not really such a big problem?

@albertz
Copy link
Member Author

albertz commented Feb 15, 2024

Getting back to this, for completeness, some new alternatives:

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

No branches or pull requests

2 participants