Fix TypeError in keras.ops.searchsorted by using shape instead of len #21472
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @dhantule, 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 TypeError in the keras.ops.searchsorted function, making it robust and compatible with symbolic Tensors. The core change involves updating how the size of the input sequence is determined, moving from a direct len() call to accessing the shape attribute, which is crucial for operations involving symbolic graph computations.
Highlights
- Bug Fix: TypeError with Symbolic Tensors: I've addressed a
TypeErrorthat occurred inkeras.ops.searchsortedwhen thesorted_sequenceargument was a symbolic Tensor. The previous implementation usedlen(sorted_sequence)to determine the output type, which is not compatible with symbolic Tensors. - Compatibility with Symbolic Tensors: The fix ensures that
searchsortedcan now correctly handle symbolic Tensors by replacing the problematiclen()call withsorted_sequence.shape[0], which is the appropriate way to get the size of the first dimension for a Tensor.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and 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 to provide feedback.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request fixes a TypeError in keras.ops.searchsorted for symbolic tensors by using sorted_sequence.shape[0] instead of len(sorted_sequence). The feedback focuses on improving the readability of the changed code by simplifying the ternary expression and removing redundant parentheses.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #21472 +/- ##
=======================================
Coverage 82.80% 82.80%
=======================================
Files 565 565
Lines 55505 55508 +3
Branches 8662 8662
=======================================
+ Hits 45962 45965 +3
Misses 7429 7429
Partials 2114 2114
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:
|
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
| @@ -2134,7 +2134,7 @@ def searchsorted(sorted_sequence, values, side="left"): | |||
| f"sorted_sequence.shape={sorted_sequence.shape}" | |||
| ) | |||
| out_type = ( | |||
| "int32" if len(sorted_sequence) <= np.iinfo(np.int32).max else "int64" | |||
| "int32" if sorted_sequence.shape[0] <= np.iinfo(np.int32).max else "int64" | |||
There was a problem hiding this comment.
You also need to check sorted_sequence.shape[0] is not None (it could be None for a symbolic tensor)
There was a problem hiding this comment.
Hi @fchollet, I have tested searchsorted with sorted_sequence as None and I am getting value error, traceback is shown here. Should I need to raise another value error ?
keras_sorted= keras.ops.searchsorted(None, [13, 12])
print("keras_sorted", keras_sorted)
Traceback (most recent call last):
File "/Users/dhantule/Desktop/keras-test/searchsorted#21465.py", line 22, in <module>
keras_sorted= keras.ops.searchsorted(None, [13, 12])
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/dhantule/Desktop/dhantule-keras/keras/keras/src/ops/numpy.py", line 5329, in searchsorted
sorted_sequence = backend.convert_to_tensor(sorted_sequence)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/dhantule/Desktop/dhantule-keras/keras/keras/src/backend/tensorflow/core.py", line 153, in convert_to_tensor
return tf.convert_to_tensor(x, dtype=dtype)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/tensorflow/python/util/traceback_utils.py", line 153, in error_handler
raise e.with_traceback(filtered_tb) from None
File "/Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/tensorflow/python/framework/constant_op.py", line 108, in convert_to_eager_tensor
return ops.EagerTensor(value, ctx.device_name, dtype)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ValueError: Attempt to convert a value (None) with an unsupported type (<class 'NoneType'>) to a Tensor.
There was a problem hiding this comment.
Oh the sorted_sequence would not be None, simply, it's shape would have a None dimension.
I think this is one way to trigger it is:
keras_sorted= keras.ops.searchsorted(keras.KerasTensor((None,), dtype="float32"), [13, 12])
|
Sorry, if I'm not familiar with the codebase enough, but would "backend/numpy" and "backend/torch" also need the same fix? Code looks identical there: https://github.com/search?q=repo%3Akeras-team%2Fkeras%20len(sorted_sequence)&type=code |
Yes, fixes need to be ported across backends to maintain consistent behavior |
|
@dhantule just checking in on the status of this fix, are you facing any blockers? |
Hi @JyotinderSingh, I'll port the fix across backends, just wanted to know about this |
Replaces
len(sorted_sequence)withsorted_sequence.shape[0]in thesearchsortedfunction to prevent a TypeError whensorted_sequenceis a symbolic Tensor, error reproduced in this gist.Fixes: #21465