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

Generate inference types + start using output types #2036

Merged
merged 29 commits into from
Feb 29, 2024

Conversation

Wauplin
Copy link
Contributor

@Wauplin Wauplin commented Feb 20, 2024

cc @SBrandeis @julien-c for visibility.

This PR adds scripts to generate inference types + start to use the output types in the InferenceClient.

The broader goal is to use all the spec-ed types in the InferenceClient but also in transformers and diffusers pipelines.

How it works:

Docs:

TODO:

  • add types to inference package reference in docs
  • how to handle parsing audio or images in output? currently done "manually", e.g. not from the json specs
  • fix tests
  • fix type annotations in _client.py...
  • make dataclasses first class citizen in huggingface_hub?
  • better auto-generated docstrings => following HF docs format => will be done later

Breaking changes in outputs 💔:

  • automatic_speech_recognition: str => AutomaticSpeechRecognitionOutput
  • summarization: str => SummarizationOutput
  • translation: str => TranslationOutput

Left out of scope for this PR:

  • text_generation => requires more typing for stream=True
  • deal with conversational => will be deprecated/removed soon
  • use input parameters generated from specs

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

Copy link

codecov bot commented Feb 21, 2024

Codecov Report

Attention: Patch coverage is 97.85047% with 23 lines in your changes are missing coverage. Please review.

Project coverage is 82.71%. Comparing base (e7f243c) to head (e25bcaf).

❗ Current head e25bcaf differs from pull request most recent head 56955a8. Consider uploading reports for the commit 56955a8 to get more accurate results

Files Patch % Lines
...gingface_hub/inference/_generated/_async_client.py 21.42% 22 Missing ⚠️
src/huggingface_hub/inference/_client.py 96.42% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2036      +/-   ##
==========================================
+ Coverage   80.70%   82.71%   +2.01%     
==========================================
  Files          71      102      +31     
  Lines        8519     9490     +971     
==========================================
+ Hits         6875     7850     +975     
+ Misses       1644     1640       -4     

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

Copy link
Member

@julien-c julien-c left a comment

Choose a reason for hiding this comment

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

as discussed irl, looks quite cool already!

@Wauplin Wauplin marked this pull request as ready for review February 23, 2024 16:02
@Wauplin
Copy link
Contributor Author

Wauplin commented Feb 23, 2024

This PR is now ready for review :) 🎉
I've updated the PR description accordingly. This PR will introduce breaking changes for some tasks but hopefully not too problematic. I'm planning to make a release of huggingface_hub early next week but no rush to merge this PR. It can wait the next one which would allow to implement a few more TODOs before shipping.

Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

Looks good! Clear implementation, awesome to have the types so neatly defined within the runtime.

It would be awesome to gradually move towards the same input/output definitions within transformers & diffusers pipelines.

(No need to review but FYI @sanchit-gandhi @Rocketknight1 @amyeroberts @molbap @yiyixuxu as we'll want to adopt this soon)

src/huggingface_hub/inference/_client.py Outdated Show resolved Hide resolved
utils/generate_inference_types.py Show resolved Hide resolved
utils/helpers.py Show resolved Hide resolved
Comment on lines 36 to 43
@dataclass
class TextClassificationOutput(BaseInferenceType):
"""Outputs of inference for the Text Classification task"""

label: str
"""The predicted class label."""
score: float
"""The corresponding probability."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Output is actually a List of those if I'm not mistaken :(

I had to manually parse the TS code to add that array definition in huggingface.js (bug also reported in quicktype)

Copy link
Contributor Author

@Wauplin Wauplin Feb 28, 2024

Choose a reason for hiding this comment

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

Yeah I already had a script to handle the shared classes (typically ClassificationOutput) but now I pushed 56955a8 so that classes are named AudioClassificationOutputElement, to emphasize that the expected output will be a list of those.

Does it look better for you like this?

@SBrandeis
Copy link
Contributor

Very cool that the inference types are used in the Inference client right away!

@Wauplin
Copy link
Contributor Author

Wauplin commented Feb 29, 2024

Let's merge this PR now that everything's addressed! I have listed remaining todos in a follow-up issue: #2063.

@Wauplin Wauplin merged commit 3575a72 into main Feb 29, 2024
16 checks passed
@Wauplin Wauplin deleted the generate-inference-type branch February 29, 2024 16:03
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

5 participants