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

[Frontend] Support suffix in completions API (fill-in-the-middle, FIM) #9522

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

njhill
Copy link
Member

@njhill njhill commented Oct 19, 2024

For code models that have been trained with an infilling task.

Handle model-specific FIM encoding rules in a similar way to how we're handling different tool parsers.

For example, can be enabled for for Codestral by adding command line arg --fim mistral (as well as --tokenizer-mode mistral in this case).

@njhill
Copy link
Member Author

njhill commented Oct 19, 2024

Planning to simplify this, would be better for it to work more like chat templates than tool parsers.

@mgoin
Copy link
Member

mgoin commented Oct 20, 2024

This is exciting! Looking forward to review

Handle model-specific FIM encoding rules in a similar way to how we're handling different tool parsers.
@njhill njhill force-pushed the fim branch 2 times, most recently from 0b1e762 to 37135ae Compare October 22, 2024 00:10
@vllm-project vllm-project deleted a comment from github-actions bot Oct 22, 2024
@njhill njhill force-pushed the fim branch 2 times, most recently from c4886c3 to 4a2e7e9 Compare October 22, 2024 00:55
@njhill
Copy link
Member Author

njhill commented Oct 23, 2024

FYI @patrickvonplaten if you'd like to check the Mistral part. Is there a small Mistral model on the HF hub that we could use for this in tests?

@patrickvonplaten
Copy link
Contributor

I guess codestral might be too big? https://huggingface.co/mistralai/Codestral-22B-v0.1

The 7B-v0.3 should also work reasonably well for FIM though: https://huggingface.co/mistralai/Mistral-7B-Instruct-v0.3

@njhill
Copy link
Member Author

njhill commented Oct 24, 2024

Thanks @patrickvonplaten, yes I was hoping for something smaller than codestral 22B ... will try with the 7B thanks!

# Conflicts:
#	vllm/entrypoints/openai/api_server.py
#	vllm/entrypoints/openai/serving_completion.py
#	vllm/entrypoints/openai/serving_embedding.py
#	vllm/entrypoints/openai/serving_engine.py
@mergify mergify bot added the frontend label Nov 5, 2024
Copy link

mergify bot commented Nov 15, 2024

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @njhill.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify bot added the needs-rebase label Nov 15, 2024
@thomasbs17
Copy link

Hi, this would be a great addition to continue.dev.
I was wondering if there was any update on reviewing/merging this PR?

@gadikar-deshaw
Copy link

Hi, following this as well.
This feature would be a great addition. Is there any plan to merge this PR?

Thanks!

@njhill
Copy link
Member Author

njhill commented Feb 12, 2025

@thomasbs17 @gadikar-deshaw apologies, I've been diverted from this by other priorities but will try to resurrect it when I can.

Any help would be welcome - apart from bringing the existing changes up to date I think the only thing remaining was adding tests.

@thomasbs17
Copy link

@thomasbs17 @gadikar-deshaw apologies, I've been diverted from this by other priorities but will try to resurrect it when I can.

Any help would be welcome - apart from bringing the existing changes up to date I think the only thing remaining was adding tests.

Sounds good, thanks! I'll try to look into adding the tests

@CZH-THU
Copy link

CZH-THU commented Feb 13, 2025

Hi, when this PR can be merged into main?

@thomasbs17
Copy link

@njhill I've worked on bringing your branch up to speed with main. PR is here

@mgoin mgoin changed the title [Frontend] Support suffix in completions API (fill-in-the-middle) [Frontend] Support suffix in completions API (fill-in-the-middle, FIM) Feb 17, 2025
@njhill
Copy link
Member Author

njhill commented Feb 17, 2025

Thank you @thomasbs17! I'll try to look at this today.

Co-authored-by: Thomas Bouamoud <[email protected]>
@mergify mergify bot removed the needs-rebase label Feb 18, 2025
Signed-off-by: Nick Hill <[email protected]>
@njhill
Copy link
Member Author

njhill commented Feb 18, 2025

Thanks @thomasbs17! I have merged your main-merge, and another commit to fix the linting errors.

Signed-off-by: Nick Hill <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants