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

feat: add support for multimodal in Vertex #1338

Merged
merged 6 commits into from
Sep 9, 2024
Merged

feat: add support for multimodal in Vertex #1338

merged 6 commits into from
Sep 9, 2024

Conversation

ArthurGoupil
Copy link
Contributor

@ArthurGoupil ArthurGoupil commented Jul 10, 2024 •
edited
Loading

@adhishthite
Copy link
Contributor

@ArthurGoupil This is awesome. Will the same multimodal principles apply for OpenAI endpoints too? I am looking to develop multimodal capability for OpenAI endpoints

@ArthurGoupil
Copy link
Contributor Author

Hi @adhishthite, I think you can reuse the generic parts of the implementation in #1021, but the final data structure (with inlineData) is specific to Vertex, you should find the good one in OpenAI doc

@nsarrazin nsarrazin added enhancement New feature or request back This issue is related to the Svelte backend or the DB models This issue is related to model performance/reliability labels Jul 12, 2024
@nsarrazin nsarrazin self-assigned this Jul 16, 2024
nsarrazin
nsarrazin reviewed Jul 16, 2024
View reviewed changes
Copy link
Collaborator

@nsarrazin nsarrazin left a comment

Choose a reason for hiding this comment

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

Hi, thanks for the contribution, looks great!

Would it be possible to also update the docs here to show how we can use multimodal models with vertex ?

Other than that LGTM!

@nsarrazin nsarrazin removed their assignment Jul 16, 2024
pocman
pocman reviewed Jul 17, 2024
View reviewed changes
Copy link
Contributor

@pocman pocman left a comment

Choose a reason for hiding this comment

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

Tested locally, it's working, I'm able to upload an image and the model is using it to answer.

README.md Outdated Show resolved Hide resolved
README.md Outdated
"maxWidth": 2000,
"maxHeight": number;
}
}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}"
}

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/lib/server/endpoints/google/endpointVertex.ts Outdated Show resolved Hide resolved
@nsarrazin
Copy link
Collaborator

Hi @ArthurGoupil, I'm not super familiar with the vertex endpoint but wdyt of the comments on the review from @pocman ? Feel free to approve or not and then I'll merge this 😄

@ArthurGoupil
Copy link
Contributor Author

Hi @nsarrazin, yeah i need to apply them once i have some time, probably in september :/

pocman
pocman suggested changes Sep 5, 2024
View reviewed changes
src/lib/server/endpoints/google/endpointVertex.ts Outdated
@@ -73,7 +92,8 @@ export function endpointVertex(input: z.input<typeof endpointVertexParametersSch
stopSequences: parameters?.stop,
temperature: parameters?.temperature ?? 1,
},
tools,
// tools and multimodal are mutually exclusive
tools: !multimodal ? tools : undefined,
Copy link
Contributor

@pocman pocman Sep 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

condition should not be on !multimodal (otherwise, the model without picture provided will never use the other tools) but on files.length == 0.

If not files is provided for this specific query to vertex, then tools can be provided

@pocman
Copy link
Contributor

pocman commented Sep 5, 2024

Working as expected, the tools array is not added to the query only if a picture was provided, otherwise tools are available.

Screenshot 2024-09-05 at 15 15 23 Screenshot 2024-09-05 at 15 15 37

@pocman
Copy link
Contributor

pocman commented Sep 9, 2024

@nsarrazin I believe this is ready to be merged. Can you have a look ?

@nsarrazin
Copy link
Collaborator

Thanks a lot @ArthurGoupil @pocman 🔥 Merging this now

@nsarrazin nsarrazin merged commit 1038d74 into huggingface:main Sep 9, 2024
3 checks passed
ice91 pushed a commit to ice91/chat-ui that referenced this pull request Oct 30, 2024
@ArthurGoupil
feat: add support for multimodal in Vertex (huggingface#1338)
9549e2b 
* feat: add support for multimodal in Vertex

* Nit changes and remove  tools if multimodal

* revert model name change

* Fix tools/multimodal condition

* chores(lint): fix formatting

---------
Co-authored-by: Thomas <thomas.poc@gmail.com>
Co-authored-by: Nathan Sarrazin <sarrazin.nathan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nsarrazin nsarrazin nsarrazin left review comments

@pocman pocman pocman requested changes

Assignees
No one assigned
Labels
back This issue is related to the Svelte backend or the DB enhancement New feature or request models This issue is related to model performance/reliability
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ArthurGoupil @adhishthite @nsarrazin @pocman