-
Notifications
You must be signed in to change notification settings - Fork 26.8k
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 Kosmos2Processor
batch mode
#27323
Conversation
@@ -211,7 +211,9 @@ def __call__( | |||
image_embeds_position_mask.append(mask) | |||
|
|||
if isinstance(text, list): | |||
sorted_length = sorted([(idx, len(x)) for idx, x in enumerate(text_encoding.input_ids)]) | |||
sorted_length = sorted( | |||
[(idx, len(x)) for idx, x in enumerate(text_encoding.input_ids)], key=lambda x: x[-1] |
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.
Here my intention was to sort with the length, not the index. However, I forgot to specify the key
so it did not do what I expected.
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.
Note that we do have tests regarding the batch mode - but the examples I used have the longest sequence as the last example in a batch, so it didn't detect this issue 😭
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.
Can we update the test so that it would have been caught?
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.
OK!
All tests (even slow) pass! |
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 fixing this!
The documentation is not available anymore as the PR was closed or merged. |
* fix * fix * fix --------- Co-authored-by: ydshieh <[email protected]>
What does this PR do?
Fix
Kosmos2Processor
batch mode as there is a bug, see comment in the code change.Fix one issue about batch mode opened in #27301