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

Make chat templates part of ProcessorMixin #30744

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

Conversation

Rocketknight1
Copy link
Member

Right now, IDEFICS2 supports chat templates, but in class-specific code. In this PR, we'll try to move that support to the generic ProcessorMixin so that we don't have to keep duplicating it as more VLMs (like LLaVA) start to need chat templates.

Draft PR for now until I make sure this doesn't break things!

@molbap
Copy link
Contributor

molbap commented May 10, 2024

Hi @Rocketknight1, very nice! FYI, I'm working on standardizing the processors here #30511 and I was thinking how to make this compatible with apply_chat_template - thought you might want to take a look at the design here, and I'll keep an eye here as well :)

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@Rocketknight1
Copy link
Member Author

@molbap I didn't see your PR, but we should definitely use your approach! Supporting chat templates should be simple enough - all they require is moving apply_chat_template to ProcessorMixin, and then supporting chat_template as an attribute for any processor that wants to use them. Do you want me to pause my PR until yours is merged?

@molbap
Copy link
Contributor

molbap commented May 10, 2024

I don't think you need to pause yours! I'll try and get mine merged soon - it's already approved, just need to make sure nothing breaks

@Rocketknight1
Copy link
Member Author

Cool! I'll probably wait until yours is merged, in that case, and then adding chat templates should be clean and straightforward.

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.

None yet

3 participants