microsoft
.Net: Fix hugging face embedding
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 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
- [x] The code builds clean without any errors or warnings
- [x] The PR follows the SK Contribution Guidelines and the pre-submission formatting script raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone :smile:
Hello @dmytrostruk Can you please suggest to me where I can add the above custom HttpClient Example in main code .
Hello @dmytrostruk Can you please suggest to me where I can add the above custom HttpClient Example in main code .
@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: https://github.com/microsoft/semantic-kernel/blob/main/dotnet/samples/Concepts/Memory/HuggingFace_EmbeddingGeneration.cs
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 Added my example in https://github.com/microsoft/semantic-kernel/commit/4f9eb974f6480c92679a25c4579de2244ceb2a32 . Please merge my PR.. Let me know for anymore Improvement.
@RogerBarreto It might be possible that changes in TextEmbeddingResponse class will lead to test case failure
@N-E-W-T-O-N there are still formatting errors to be addressed. Suggest you run dotnet format on the solution.
@dmytrostruk please check it now. Resolve check-format issues .Sorry for inconvenience
may I ask how to test unit test cases locally
@N-E-W-T-O-N
cd dotnet/src/Connectors/Connectors.HuggingFace.UnitTests
dotnet test
@dmytrostruk The issue is unit tests are still based on the previous version of embedding dimensions as noted by@RogerBarreto .Would you like me to update the unit test accordingly?
The issue is unit tests are still based on the previous version of embedding dimensions as noted by@RogerBarreto .Would you like me to update the unit test accordingly?
@N-E-W-T-O-N Yes, if we are changing the behavior, then unit tests should be updated as well. Thanks!
@dmytrostruk moved the class inside with access modifier 'private sealed'
Resolve the unit test issue
@dmytrostruk anything to add from my side
@N-E-W-T-O-N I think we are ready to merge this to main. I added a couple of small fixes to the example. Thanks for your contribution!