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.

Sign up for GitHub

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

Jump to bottom

Make Sampler a member of the class for llm/vlm pipelines #1347

Merged
merged 2 commits into from
Dec 10, 2024
Merged

Make Sampler a member of the class for llm/vlm pipelines #1347

merged 2 commits into from
Dec 10, 2024

Conversation

sbalandi
Copy link
Contributor

@sbalandi sbalandi commented Dec 9, 2024 •
edited
Loading

For fix wrong behavior in case of the random sampling #1341

@github-actions github-actions bot added category: visual language Visual language pipeline category: LLM LLM pipeline (stateful, static) labels Dec 9, 2024
@sbalandi sbalandi requested review from Wovchena and ilya-lavrenov and removed request for Wovchena December 9, 2024 15:28
@sbalandi sbalandi mentioned this pull request Dec 9, 2024
Merged
@ilya-lavrenov ilya-lavrenov linked an issue Dec 9, 2024 that may be closed by this pull request
Sampling method for LLM output tokens does not sample from random distribution #1341
Closed
ilya-lavrenov
ilya-lavrenov reviewed Dec 9, 2024
View reviewed changes
src/cpp/src/visual_language/pipeline.cpp Outdated Show resolved Hide resolved
src/cpp/src/llm_pipeline.cpp Outdated Show resolved Hide resolved
@ilya-lavrenov ilya-lavrenov added this to the 2024.6 milestone Dec 9, 2024
@ilya-lavrenov ilya-lavrenov added the bug Something isn't working label Dec 9, 2024
Wovchena
Wovchena approved these changes Dec 10, 2024
View reviewed changes
Copy link
Collaborator

@Wovchena Wovchena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with Ilya's comments, the rest LGTM

@github-actions github-actions bot added the category: sampling Sampling / Decoding algorithms label Dec 10, 2024
@sbalandi
Copy link
Contributor Author

made clear_request at the end of each generation , also for the chat

@ilya-lavrenov ilya-lavrenov added this pull request to the merge queue Dec 10, 2024
@ilya-lavrenov ilya-lavrenov added the port to master PR needs to be ported to master from release branch label Dec 10, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 10, 2024
@ilya-lavrenov ilya-lavrenov merged commit e2a02f3 into openvinotoolkit:releases/2024/6 Dec 10, 2024
52 checks passed
@ilya-lavrenov
Copy link
Contributor

Please, port to master

@sbalandi sbalandi mentioned this pull request Dec 19, 2024
Merged
@sbalandi
Copy link
Contributor Author

Please, port to master

ported to master in #1412

@ilya-lavrenov ilya-lavrenov removed the port to master PR needs to be ported to master from release branch label Dec 19, 2024
github-merge-queue bot pushed a commit that referenced this pull request Dec 20, 2024
@sbalandi
Make Sampler a member of the class for llm/vlm pipelines (#1412)
4d18f8b 
cherry-pick #1347
to master
ScottZhang812 pushed a commit to ScottZhang812/_openvino.genai that referenced this pull request Dec 23, 2024
@sbalandi
Make Sampler a member of the class for llm/vlm pipelines (#1412)
9a53c85 
cherry-pick openvinotoolkit/openvino.genai#1347
to master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Wovchena Wovchena Wovchena approved these changes

@ilya-lavrenov ilya-lavrenov ilya-lavrenov approved these changes

Assignees

@ilya-lavrenov ilya-lavrenov

@Wovchena Wovchena

Labels
bug Something isn't working category: LLM LLM pipeline (stateful, static) category: sampling Sampling / Decoding algorithms category: visual language Visual language pipeline Code Freeze
Projects
None yet
Milestone
2024.6
Development

Successfully merging this pull request may close these issues.

Sampling method for LLM output tokens does not sample from random distribution
4 participants
@sbalandi @ilya-lavrenov @Wovchena @moslex