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

Support TF 2.15 #101

Merged
merged 14 commits into from
Jan 8, 2024
Merged

Support TF 2.15 #101

merged 14 commits into from
Jan 8, 2024

Conversation

khurram-ghani
Copy link
Collaborator

@khurram-ghani khurram-ghani commented Jan 5, 2024

Added support for tensorflow 2.15.

Required fixing of two issues:

  1. Certain combinations of mypy, python and tensorflow (due to typing-extensions) do not work, resulting in the error ImportError: cannot import name 'Self' from 'typing_extensions' .... Worked around by limiting mypy version for older python versions.
  2. The behaviour of tensorflow.python.util.object_identity.ObjectIdentitySet has changed such that the set equality now compares the internal wrappers instead of the wrapped objects (see tensorflow/tensorflow@bc28335). This resulted in a failing test. The test has been updated to directly compare the object ids, instead of using the internal tensorflow class. Note: this class is also used in optimization/keras_natgrad.py, but that behaviour hasn't changed and should continue to work. So that has been left alone in this PR.

Copy link
Collaborator

@uri-granta uri-granta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! (just one question re TF 2.13 and 2.14)

@@ -22,7 +22,7 @@ jobs:
strategy:
matrix:
python-version: ["3.7", "3.8", "3.9", "3.10"]
tensorflow: ["~=2.5.0", "~=2.6.0", "~=2.7.0", "~=2.8.0", "~=2.9.0", "~=2.10.0", "~=2.11.0", "~=2.12.0"]
tensorflow: ["~=2.5.0", "~=2.6.0", "~=2.7.0", "~=2.8.0", "~=2.9.0", "~=2.10.0", "~=2.11.0", "~=2.12.0", "~=2.15.0"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we not want to test on the intermediate values too?

Copy link
Collaborator Author

@khurram-ghani khurram-ghani Jan 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't sure, thought that would be a lot of combinations. I'll go ahead and add it.

@khurram-ghani khurram-ghani merged commit 83dafb0 into develop Jan 8, 2024
35 checks passed
@khurram-ghani khurram-ghani deleted the khurram/support_tf_2_15 branch January 8, 2024 14:35
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.

None yet

2 participants