-
Notifications
You must be signed in to change notification settings - Fork 19.7k
Fix ops.tile shape inference issue on TensorFlow backend #21860
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
base: master
Are you sure you want to change the base?
Conversation
When ops.tile is called inside a Layer's call method with concrete integer repeats, the TensorFlow backend was converting those repeats to a tensor, which prevented TensorFlow's shape inference from properly determining the output shape. This resulted in all-None shapes. Changes: 1. Modified TensorFlow backend's tile() to detect when repeats contains only concrete integer values and pass them directly to tf.tile as a Python list/tuple instead of converting to a tensor. This allows TensorFlow's shape inference to work correctly. 2. Enhanced ops.numpy.Tile.compute_output_spec() to handle symbolic repeat values more gracefully by checking if each repeat is a concrete integer before attempting multiplication. 3. Added regression tests to verify shape inference works correctly both in direct ops.tile calls and when used inside Layer.call(). Fixes keras-team#20914 Signed-off-by: Samaresh Kumar Singh <[email protected]>
Summary of ChangesHello @ssam18, 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 resolves a critical shape inference 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 effectively addresses the shape inference issue with ops.tile on the TensorFlow backend, particularly when used within a Layer. The approach of detecting concrete integer repeats and passing them as a Python list to tf.tile is a solid solution that correctly enables static shape inference. The corresponding adjustments in Tile.compute_output_spec to handle symbolic repeats more gracefully are also well-implemented. The inclusion of targeted regression tests is a great addition that ensures the fix is robust and prevents future regressions.
My only suggestion is to refine the exception handling in the TensorFlow backend tile function to be more specific, which will improve maintainability and debugging.
Signed-off-by: Samaresh Kumar Singh <[email protected]>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #21860 +/- ##
==========================================
+ Coverage 82.47% 82.56% +0.08%
==========================================
Files 577 577
Lines 59508 59597 +89
Branches 9332 9355 +23
==========================================
+ Hits 49080 49206 +126
+ Misses 8015 7982 -33
+ Partials 2413 2409 -4
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:
|
Run api_gen.py to regenerate API directory with properly formatted imports and ordering. This is required by the pre-commit hook. Signed-off-by: Samaresh Kumar Singh <[email protected]>
Summary
Fixes issue #20914 where
ops.tilereturns all-None shapes when called inside a Layer's call method with concrete integer repeats on the TensorFlow backend.Problem
When
ops.tilewas called inside a Layer with concrete integer repeats like[1, 2, 1, 1], the TensorFlow backend was converting those repeats to a tensor, which prevented TensorFlow's shape inference from properly determining the output shape. This resulted in all dimensions beingNone.For example:
Solution
TensorFlow Backend: Modified
tile()to detect when repeats contains only concrete integer values and pass them directly totf.tileas a Python list instead of converting to a tensor. This allows TensorFlow's shape inference to work correctly.Symbolic Ops: Enhanced
Tile.compute_output_spec()to handle symbolic repeat values more gracefully by checking if each repeat is a concrete integer before attempting multiplication.Tests: Added regression tests to verify shape inference works correctly both in direct
ops.tilecalls and when used insideLayer.call().Testing
NumpyOneInputOpsDynamicShapeTest.test_tilefor multi-dimensional inputstest_tile_shape_inference_in_layerspecifically for the Layer use caseFixes #20914