Skip to content

[inference] Necessary breaking change: nest task-specific route inside of model route #3044

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

Merged
merged 3 commits into from
May 5, 2025

Conversation

julien-c
Copy link
Member

@julien-c julien-c commented May 2, 2025

The alternative is just to break this feature...

@julien-c julien-c requested a review from oOraph May 2, 2025 11:50
Copy link

codecov bot commented May 2, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.00%. Comparing base (37ab5ba) to head (5fdf61e).
Report is 17 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #3044       +/-   ##
===========================================
+ Coverage   43.75%   81.00%   +37.24%     
===========================================
  Files         124      124               
  Lines       12261    12430      +169     
===========================================
+ Hits         5365    10069     +4704     
+ Misses       6896     2361     -4535     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@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.

@julien-c julien-c marked this pull request as ready for review May 5, 2025 07:30
@hanouticelina
Copy link
Contributor

Just to make sure I understand from the context of this internal thread: the goal here is to point feature-extraction and sentence-similarity tasks to the same model route so that the same TEI deployment can answer both kinds of calls, right? (instead of having two deployments of the exact same model under its own /pipeline).
The PR looks good but let's wait for the update server-side (iiuc the PR won't work without switching to the TEI container).

@julien-c
Copy link
Member Author

julien-c commented May 5, 2025

the goal here is to point feature-extraction and sentence-similarity tasks to the same model route so that the same TEI deployment can answer both kinds of calls

not necessarily TEI, but yeah, the same container would be able to answer both calls/tasks, unlike now where we need two (redundant) running containers (unless we'd make complex URL/route rewriting)

Can you confirm this @oOraph?

@hanouticelina
Copy link
Contributor

I added unit tests and tested the PR 👍 all good, let's merge this (the inference tests are passing).

from huggingface_hub import InferenceClient


client = InferenceClient(provider="hf-inference")

similarities = client.sentence_similarity(
    model="intfloat/e5-base-v2",
    sentence="Hello, how are you?",
    other_sentences=["Hello, how are you?", "I am fine, thank you!"],
)

print(similarities)

@hanouticelina hanouticelina merged commit 83ba43c into main May 5, 2025
24 of 25 checks passed
@hanouticelina hanouticelina deleted the change-pipeline-support branch May 5, 2025 09:17
julien-c added a commit to huggingface/huggingface.js that referenced this pull request May 5, 2025
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