feat(llms): support more tasks in HuggingFaceHub LLM and remove deprecated dep

[mspronesti]

• Description: this PR upgrades the HuggingFaceHub LLM:
  • support more tasks (translation and conversational)
  • replaced the deprecated InferenceApi with InferenceClient
  • adjusted the overall logic to use the "recommended" model for each task when no model is provided, and vice-versa.
• Tag mainter(s): @baskaryan @hwchase17

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
langchain	⬜️ Ignored (Inspect)	Visit Preview		Jan 19, 2024 10:35am

[mspronesti]

@hwchase17 @baskaryan @efriis Could you please review this? I can drop these changes if they don't look good to you, but I suppose the upgrade to the newest version of hf hub API is needed (part of this PR)😄

[mspronesti]

@baskaryan @hwchase17 any chance this gets reviewed?

[Wauplin]

Hi @mspronesti 👋 Here's @Wauplin, maintainer of the huggingface_hub library. I came across this PR and wanted to provide some high-level feedback on it. I'm not an expert in langchain but this solution looks definitely good. It's nice to upgrade to InferenceClient which should be much more robust and versatile (can work with a self-deployed TGI server for instance).

I spotted one part that could be improved to handle better the inputs and outputs. Otherwise, looks good to me. Feel free to ping me if you need any further assistance.

[Wauplin]

Here I would use the task-specific methods instead of client.post directly. For the record, client.post(...) is what's used internally by the client to make the calls. It is perfectly valid to use it but you also have higher-level method -like client.text_generation(...)- that are more appropriate to handle inputs and outputs correctly. See supported tasks and their documentation on this page.

So instead of those two lines, I would suggest something like

if self.task == "text-generation":
    response = client.text_generation(...)
elif self.task == "conversational"
    response = client.conversational(...)
...

Inputs and outputs may vary depending on the selected task. It would also avoid the logic if isinstance(response, list): ... else: ... below that might be error prone.

[mspronesti]

Hi @Wauplin, thanks for your precious feedback! I had a look at the InferenceClient implementation in hf-hub, but I couldn't find any method handling 'text2text-generation'. This task is still supported by post though.

Does this porting to the newer higher-level API look good to you?

        if self.task == "text-generation":
            response = self.client.text_generation(prompt, **params)
        elif self.task == "conversational":
            response = self.client.conversational(prompt, **params)["generated_text"]
        elif self.task == "translation":
            response = self.client.translation(prompt, **params)
        elif self.task == "summarization":
            response = self.client.summarization(prompt, parameters=params)
        else:
            raise ValueError(f"Invalid task {self.task}")

[Wauplin]

Looks good to me yes! It should be more robust as well. Thanks for making the change :)

About text2text-generation, we are trying to deprecate it in favor of text-generation (for harmonization) cc @osanseviero. I think that using self.client.text_generation should be fine but would be best to test it first. According to the docs, the API should be the same (see here).

[osanseviero]

The API is the same, but the underlying pipelines in transformers are different (one is encoder-decoder while the other is decoder-only), so not sure they would work out of the box.

[mspronesti]

@hwchase17 @baskaryan Any thoughts/suggestions on this?

[mspronesti]

@hwchase17 Could you please review these changes? If the newest tasks are not needed, I can drop them. I believe that the upgrade to hf API introduced by this PR would be needed though.

[hwchase17]

thanks!

[hwchase17]

@mspronesti just so i understand before merging in, the only hesitation from the HF team comes from whether to use post or the SDK methods? and all other things look good?

for post vs SDK, the issue is one task isnt supported on the SDK methods? If that's correct, I don't have too strong of a feeling either way

just wanted to make sure i fully understand before merging in!

[mspronesti]

@hwchase17 I believe that's correct. To keep full backward compatibility with the previously supported tasks, I'd keep the current version (with post). The SDK methods call internally post, thus I believe there's no big difference (except they are less error-prone from the perspective of the programmer). Any of the two alternatives works for me though, so please pick one and I will update the PR accordingly if needed!

[Wauplin]

To keep full backward compatibility with the previously supported tasks, I'd keep the current version (with post). The SDK methods call internally post, thus I believe there's no big difference.

Agree with you here! Keeping the PR as it is now is good :)

except they are less error-prone from the perspective of the programmer

Yes, that's the main point of using the other methods based on .post so no big deal not using them here.

[mspronesti]

Thank you for confirming @Wauplin! @hwchase17 I suppose this is ready for merge then, unless you have additional observations/suggestions :)

[hwchase17]

merged! thank you @mspronesti @Wauplin , and sorry for the delay!