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

Add support for py312, drop support for py38 #61

Merged
merged 9 commits into from
Oct 1, 2024

Conversation

wbenoit26
Copy link

@wbenoit26 wbenoit26 commented Sep 26, 2024

This PR expands the python version range to include 3.12. As a consequence, the TensorFlow version range has to be changed to include 2.16, as it's the first version that includes support for python 3.12. However, TensorFlow 2.16 is also the first version that uses Keras 3 by default. I'm not sure how feasible it is to support both Keras 2 and 3, or whether it would be worth the effort of doing so, but it's possible to set an environment variable that forces the use of Keras 2.

Edit: And to use the tf-keras package that allows the continued use of Keras 2, python 3.8 can no longer be supported.

@wbenoit26 wbenoit26 changed the title Relax python version Add support for py312, drop support for py38 Sep 26, 2024
@wbenoit26
Copy link
Author

@EthanMarx This should be good to go, let me know what you think.

@EthanMarx
Copy link
Contributor

What is the reason we need to add tf-keras as a dep? can we get away with not specifying it and letting users decide which version they use? I'm not as familiar with Keras

@wbenoit26
Copy link
Author

Using Keras 3 broke some of the tests - it seems to have changed the way layer names get used, and I'm not sure there's a syntax that works for both Keras 2 and 3, though worth looking into in the future.

The tf_keras package is just Keras 2.15 with a different package name (see point 1 under "Moving from Keras 2 to Keras 3" here), and it's how Tensorflow recommends using Keras 2 with Tensorflow 2.16.

@EthanMarx
Copy link
Contributor

Okay got it - so migrating to Keras 3, will break the export. Definitely makes sense to hard enforce keras 2 then.

@wbenoit26 wbenoit26 merged commit 43924c4 into ML4GW:dev Oct 1, 2024
5 checks passed
@wbenoit26 wbenoit26 deleted the relax-python-version branch October 1, 2024 13:44
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

Successfully merging this pull request may close these issues.

2 participants