-
Notifications
You must be signed in to change notification settings - Fork 200
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
Use whole history in case of undetermined tokenization of sequence #1254
Conversation
49a4c9b
to
05b3302
Compare
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.
Not for this PR, but we need tests for that change. I remember you said random miniCPM diverges. This PR probably fixes that, so your case can be used as a test for VLM. LLM needs its test as well, maybe you can provoke a random weights model to generate such tokens.
comments have been addressed - #1268 |
Let's sync with #1268 and address remaining comments from that PR if they are valid |
ddd706c
to
266c4b6
Compare
src/cpp/src/utils.cpp
Outdated
ov::Tensor old_tensor = state.get_state(); | ||
// [BATCH_SIZE, num_kv_heads, seq_len, head_size] | ||
auto shape = old_tensor.get_shape(); | ||
shape[2] -= remove_from_end; |
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.
Unfortunately, sequence is represented by another dimension sometimes. See
openvino.genai/samples/cpp/prompt_lookup_decoding_lm/prompt_lookup_decoding_lm.cpp
Line 116 in d189eb7
shape[seq_len_axis] = new_seq_len; |
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 point, added runtime search seq_len_axis
c12aee5
to
05305e3
Compare
Task: CVS-157295