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

Support Qwen in model builder #739

Merged
merged 3 commits into from
Aug 1, 2024
Merged

Conversation

BowenBao
Copy link
Contributor

@BowenBao BowenBao commented Jul 31, 2024

Resolves #718

cc @spandantiwari-amd

@BowenBao
Copy link
Contributor Author

cc @kunal-vaishnavi for review

@kunal-vaishnavi
Copy link
Contributor

Thanks for the contribution! Can you also update the following places?

  1. Add the qwen2 model type that will show up in genai_config.json here
  2. Add Qwen to the README here to show that Qwen is now supported

@BowenBao
Copy link
Contributor Author

Thanks @kunal-vaishnavi, updated.

I realized we will need to introduce the Qwen tokenizer as well for it to run e2e in OGA. I'm putting Qwen in "Under Development" column for now.

@yufenglee
Copy link
Member

Thanks @kunal-vaishnavi, updated.

I realized we will need to introduce the Qwen tokenizer as well for it to run e2e in OGA. I'm putting Qwen in "Under Development" column for now.

What tokenizer does Qwen use? Does it use same tokenizer as llama?

@yufenglee
Copy link
Member

Thanks @kunal-vaishnavi, updated.
I realized we will need to introduce the Qwen tokenizer as well for it to run e2e in OGA. I'm putting Qwen in "Under Development" column for now.

What tokenizer does Qwen use? Does it use same tokenizer as llama?

Synced up with @wenbingl. There needs a small change to support Qwen tokenizer. He will add the change.

@wenbingl
Copy link
Member

Thanks @kunal-vaishnavi, updated.
I realized we will need to introduce the Qwen tokenizer as well for it to run e2e in OGA. I'm putting Qwen in "Under Development" column for now.

What tokenizer does Qwen use? Does it use same tokenizer as llama?

Synced up with @wenbingl. There needs a small change to support Qwen tokenizer. He will add the change.

a quick PR is here: microsoft/onnxruntime-extensions#781

@BowenBao
Copy link
Contributor Author

Thank you for the quick response @wenbingl, @yufenglee! I have validated your fix enables the qwen generation e2e.

Should I wait for the PR in extension to merge, and then include the commit update in this PR?

@kunal-vaishnavi
Copy link
Contributor

Should I wait for the PR in extension to merge, and then include the commit update in this PR?

Yes, let's wait so that Qwen can be supported end-to-end with this PR.

@wenbingl
Copy link
Member

wenbingl commented Aug 1, 2024

Thank you for the quick response @wenbingl, @yufenglee! I have validated your fix enables the qwen generation e2e.

Should I wait for the PR in extension to merge, and then include the commit update in this PR?

the PR in ort-extensions was merged.

@BowenBao
Copy link
Contributor Author

BowenBao commented Aug 1, 2024

Updated. @kunal-vaishnavi PTAL.

@yufenglee yufenglee merged commit 1fb20f8 into microsoft:main Aug 1, 2024
11 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants