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

fix(integrations): make Keras WandbCallback compatible with TF version >= 2.11.0 #4533

Merged
merged 7 commits into from Nov 30, 2022

Conversation

ayulockin
Copy link
Member

@ayulockin ayulockin commented Nov 24, 2022

Fixes WB-11561

Description

In TensorFlow 2.9, an experimental version of the new Keras Optimizer API, tf.keras.optimizers.experimental was released. It was done to provide a more unified and expanded catalog of built-in optimizers which can be more easily customized and extended.

In TensorFlow 2.11, these new optimizers are on by default.

Our Keras WandbCallback, log_gradient feature creates a _CustomOptimizer by subclassing old optimizers API.

This PR creates the _CustomOptimizer using the correct parent class. This solves the problem of log_gradient feature not working with TF 2.11.0.

Testing

This was manually tested with TF version 2.8.0, 2.9.0, 2.10.0, and 2.11.0. The W&B run pages are linked below:

Checklist

  • Include reference to internal ticket "Fixes WB-NNNN" and/or GitHub issue "Fixes #NNNN" (if applicable)
  • Ensure PR title compliance with the conventional commits standards

@codecov
Copy link

codecov bot commented Nov 24, 2022

Codecov Report

❗ No coverage uploaded for pull request base (main@8c7d8b3). Click here to learn what that means.
The diff coverage is 100.00%.

❗ Current head 137fa3f differs from pull request most recent head 37f5a9f. Consider uploading reports for the commit 37f5a9f to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #4533   +/-   ##
=======================================
  Coverage        ?   83.03%           
=======================================
  Files           ?      261           
  Lines           ?    33227           
  Branches        ?        0           
=======================================
  Hits            ?    27590           
  Misses          ?     5637           
  Partials        ?        0           
Flag Coverage Δ
functest 56.66% <100.00%> (?)
unittest 73.30% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
wandb/integration/keras/keras.py 73.76% <100.00%> (ø)

@ayulockin ayulockin marked this pull request as ready for review November 24, 2022 10:15
@github-actions github-actions bot added cc-fix and removed cc-fix labels Nov 24, 2022
Copy link
Contributor

@moredatarequired moredatarequired left a comment

Choose a reason for hiding this comment

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

Thank you! Could you also roll back the changes from #4508 to ensure that 2.11 gets tested in CI?

@dmitryduev dmitryduev requested a review from a team November 28, 2022 22:37
Copy link
Member

@dmitryduev dmitryduev left a comment

Choose a reason for hiding this comment

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

Excellent, thanks a lot for the fix @ayulockin!
+1 on what @moredatarequired requested and then should be good to go in.

@dmitryduev dmitryduev added this to the sdk-2022-12.1 milestone Nov 28, 2022
@dmitryduev
Copy link
Member

mypy issue seems unrelated, will look into that separately.

#4542)

Revert "fix(sdk): pin tensorflow<2.11.0 to prevent errors caused by API changes (#4508)"

This reverts commit 7426e49.
@dmitryduev
Copy link
Member

@ayulockin @moredatarequired I merged a revert of #4508 into this branch.

@dmitryduev dmitryduev enabled auto-merge (squash) November 30, 2022 19:33
@dmitryduev dmitryduev merged commit 87f07c1 into main Nov 30, 2022
@dmitryduev dmitryduev deleted the fix-wb-11561 branch November 30, 2022 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants