-
Notifications
You must be signed in to change notification settings - Fork 4.4k
.Net: Fix hugging face embedding #6673
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
.Net: Fix hugging face embedding #6673
Conversation
Added class `TextEmbeddingResponseType1` & `TextEmbeddingResponseType2` to parse two distinct possible JsonRequest
…Async` - Added try-catch block to better handle parsing in `GenerateEmbeddingsAsync` method.
dotnet/src/Connectors/Connectors.HuggingFace/Core/Models/TextEmbeddingResponse.cs
Outdated
Show resolved
Hide resolved
|
Hello @dmytrostruk |
@N-E-W-T-O-N That's awesome, thanks for validating this idea and creating an example for this! I think you can put it here: |
|
I want to add two more parameters in the api's payload body namely 'use_cache' & 'wait_for_model' I want to send this values as addition parameters in the HuggingFaceClient https://huggingface.co/docs/api-inference/en/detailed_parameters |
|
@dmytrostruk |
dmytrostruk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@N-E-W-T-O-N Thanks for your contribution! There are some warnings about possible null references and formatting issues. In order to proceed with merging, we need to make sure that all warnings are resolved. Thanks again!
dotnet/samples/Concepts/Memory/MemoryHuggingFaceEmbedding_CustomHttpResponse.cs
Outdated
Show resolved
Hide resolved
dotnet/src/Connectors/Connectors.HuggingFace/Core/HuggingFaceClient.cs
Outdated
Show resolved
Hide resolved
dotnet/samples/Concepts/Memory/MemoryHuggingFaceEmbedding_CustomHttpResponse.cs
Outdated
Show resolved
Hide resolved
dotnet/src/Connectors/Connectors.HuggingFace/Core/HuggingFaceClient.cs
Outdated
Show resolved
Hide resolved
dotnet/src/Connectors/Connectors.HuggingFace/Core/Models/TextEmbeddingResponse.cs
Show resolved
Hide resolved
… HuggingFace_TextEmbeddingCustomHttpHandle.cs Removed trailed whitespaces & rename the file
Removed try-catch method
|
@rogerbarreto It might be possible that changes in TextEmbeddingResponse class will lead to test case failure |
|
Remove the whitespace error. |
|
@N-E-W-T-O-N there are still formatting errors to be addressed. Suggest you run |
|
@dmytrostruk anything to add from my side |
@N-E-W-T-O-N I think we are ready to merge this to |
dotnet/samples/Concepts/Memory/HuggingFace_TextEmbeddingCustomHttpHandler.cs
Outdated
Show resolved
Hide resolved
dotnet/samples/Concepts/Memory/HuggingFace_TextEmbeddingCustomHttpHandler.cs
Outdated
Show resolved
Hide resolved
|
@dmytrostruk updated the code based on the suggestion of @westey-m |
|
@dmytrostruk |
|
@dmytrostruk |
|
@dmytrostruk |
|
@N-E-W-T-O-N Thank you for contribution! |
### Motivation and Context Fix the Bug microsoft#6635 HuggingFace Embedding: Unable to Deserialization for certain models <!-- Thank you for your contribution to the semantic-kernel repo! Please help reviewers and future users providing the following information: 1. Why is this change required? 2. What problem does it solve? 3. What scenario does it contribute to? 4. If it fixes an open issue, please link to the issue here. --> ### Description As mentioned in the issue, the HuggingFace Embedding API interface returns responses typically in the form of `List<ReadOnlyMemory<float>>` and occasionally as `List<List<List<ReadOnlyMemory<float>>>>`. Currently, only the latter format is handled correctly, leading to deserialization issues. To address this, I propose the following solution: ``` try { // Attempt to parse data as List<ReadOnlyMemory<float>> and return the parsed data } catch (KernelException ex1) { try { // If the first attempt fails, attempt to parse data as List<List<List<ReadOnlyMemory<float>>>>` and return the parsed data } catch (KernelException ex2) { // If both attempts fail, handle the exception (e.g., the model doesn't exist ,the model has still been loading, or an HTTP exception occurred) and rethrow the error } } ``` ### Contribution Checklist <!-- Before submitting this PR, please make sure: --> - [x] The code builds clean without any errors or warnings - [x] The PR follows the [SK Contribution Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md) and the [pre-submission formatting script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts) raises no violations - [ ] All unit tests pass, and I have added new tests where possible - [ ] I didn't break anyone 😄 --------- Co-authored-by: Dmytro Struk <13853051+dmytrostruk@users.noreply.github.com>
Motivation and Context
Fix the Bug #6635 HuggingFace Embedding: Unable to Deserialization for certain models
Description
As mentioned in the issue, the HuggingFace Embedding API interface returns responses typically in the form of
List<ReadOnlyMemory<float>>and occasionally asList<List<List<ReadOnlyMemory<float>>>>. Currently, only the latter format is handled correctly, leading to deserialization issues.To address this, I propose the following solution:
Contribution Checklist