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

Move from tensorflow.keras to keras #2134

Open
scarlehoff opened this issue Jul 25, 2024 · 4 comments
Open

Move from tensorflow.keras to keras #2134

scarlehoff opened this issue Jul 25, 2024 · 4 comments
Labels
n3fit Issues and PRs related to n3fit

Comments

@scarlehoff
Copy link
Member

This will make the code more general. If we can remove the tensorflow dependency as much as possible (it would be impossible right now to remove it completely) that would be great (it means we could test pytorch for instance).

At the moment (tf 2.17) tensorflow.keras is still provided, but it is not a given that it will continue like that and in particular the conda forge package for macos (maybe a bug on their side, I'm not sure) seems to be missing tensorflow.keras.

@scarlehoff scarlehoff added the n3fit Issues and PRs related to n3fit label Jul 25, 2024
@RoyStegeman
Copy link
Member

RoyStegeman commented Jul 25, 2024

There is no guarantee either way, some years ago this was their position: https://github.com/keras-team/keras/releases/tag/2.3.0

I'd argue that we should keep using tensorflow.keras for possible benefits from better integration with tensorflow. If someone is serious about providing a pytorch backend in n3fit, we can reconsider and in that case swapping out tensorflow.keras for keras should be a quick job.

@scarlehoff
Copy link
Member Author

There is no guarantee either way, some years ago this was their position

I think a change of position in 5 years is still more stable than other programs we use

I've needed to add this already.
042d995
and, more generally, I think it is preferable to stick with keras which is actively being developed, that makes sure that the features we use are not deprecated (as opposed to using some tf.keras that hasn't been removed yet when it should).

@RoyStegeman
Copy link
Member

Conda packages can always be a mess, separating keras and tensorflow may also result in problems with compatibility between versions.

Feel free to do this if you want to change it, but I don't think that at this point we can say that separating them is clearly the better choice.

@scarlehoff
Copy link
Member Author

separating keras and tensorflow may also result in problems with compatibility between versions.

No. This doesn't generate (new) problems because most of the calls are already using the separate package. If you do from tensorflow import keras you are already getting the separated keras package.

But we risk using features that don't actually exist in the separated keras package (or that have changed name)
https://github.com/tensorflow/tensorflow/tree/master/tensorflow/python/keras#stop
e.g.

_to_numpy_or_python_type = tf_utils.sync_to_numpy_or_python_type

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

No branches or pull requests

2 participants