-
Notifications
You must be signed in to change notification settings - Fork 19.6k
Fix: keras.ops.quantile works with tf graph execution
#21782
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: keras.ops.quantile works with tf graph execution
#21782
Conversation
A `deque` failing to be converted to a tensor in the transpose call in `keras.ops.quantile` caused errors when running in tf graph contexts. By constructing a list from the deque before passing it to the `transpose` call we avoid this error.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello @SamKnightGit, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug in Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request addresses a bug where keras.ops.quantile would fail during TensorFlow graph execution. The issue stemmed from tf.transpose being unable to handle a collections.deque object for its permutation argument within a graph context. The fix correctly converts the deque to a list before the transpose call, resolving the error. A new test case, test_quantile_in_tf_function, has been added to specifically verify this fix by running quantile inside a @tf.function-decorated function, ensuring the operation now works as expected in graph mode. The changes are accurate and the test is well-targeted. This is a good contribution.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #21782 +/- ##
=======================================
Coverage 82.63% 82.63%
=======================================
Files 577 577
Lines 59316 59316
Branches 9300 9300
=======================================
Hits 49018 49018
Misses 7910 7910
Partials 2388 2388
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
fchollet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix!
A
dequefailing to be converted to a tensor in the transpose call inkeras.ops.quantilecaused errors when running in tf graph contexts. By constructing a list from the deque before passing it to thetransposecall we avoid this error. Test added to cover this specific case.Fixes #21781