Skip to content
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

Merged
merged 3 commits into from
Dec 16, 2024

Conversation

sbalandi
Copy link
Contributor

@sbalandi sbalandi commented Nov 26, 2024

@github-actions github-actions bot added category: visual language Visual language pipeline category: LLM LLM pipeline (stateful, static) no-match-files labels Nov 26, 2024
@sbalandi sbalandi force-pushed the tok_hist branch 2 times, most recently from 49a4c9b to 05b3302 Compare November 26, 2024 13:52
@sbalandi sbalandi marked this pull request as ready for review November 26, 2024 13:52
@ilya-lavrenov ilya-lavrenov added this to the 2025.0 milestone Nov 26, 2024
@ilya-lavrenov ilya-lavrenov added the port to LTS PR needs to be ported to LTS label Nov 26, 2024
@ilya-lavrenov ilya-lavrenov self-assigned this Nov 26, 2024
src/cpp/src/llm_pipeline.cpp Outdated Show resolved Hide resolved
src/cpp/src/llm_pipeline.cpp Outdated Show resolved Hide resolved
src/cpp/src/utils.cpp Outdated Show resolved Hide resolved
src/cpp/src/llm_pipeline.cpp Show resolved Hide resolved
src/cpp/src/visual_language/inputs_embedder.cpp Outdated Show resolved Hide resolved
src/cpp/src/visual_language/pipeline.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

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.

src/cpp/src/llm_pipeline.cpp Show resolved Hide resolved
src/cpp/src/llm_pipeline.cpp Outdated Show resolved Hide resolved
src/cpp/src/llm_pipeline.cpp Outdated Show resolved Hide resolved
src/cpp/src/llm_pipeline.cpp Show resolved Hide resolved
src/cpp/src/llm_pipeline.cpp Outdated Show resolved Hide resolved
src/cpp/src/llm_pipeline.cpp Outdated Show resolved Hide resolved
@sbalandi
Copy link
Contributor Author

sbalandi commented Nov 27, 2024

comments have been addressed - #1268

@ilya-lavrenov ilya-lavrenov removed the port to LTS PR needs to be ported to LTS label Dec 3, 2024
@ilya-lavrenov
Copy link
Contributor

Let's sync with #1268 and address remaining comments from that PR if they are valid

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;
Copy link
Collaborator

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

Copy link
Contributor Author

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

@sbalandi sbalandi force-pushed the tok_hist branch 2 times, most recently from c12aee5 to 05305e3 Compare December 15, 2024 12:29
@Wovchena Wovchena enabled auto-merge December 16, 2024 07:50
@Wovchena Wovchena added this pull request to the merge queue Dec 16, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 16, 2024
@ilya-lavrenov ilya-lavrenov added this pull request to the merge queue Dec 16, 2024
Merged via the queue into openvinotoolkit:master with commit 9e9b409 Dec 16, 2024
59 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: LLM LLM pipeline (stateful, static) category: visual language Visual language pipeline no-match-files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants