-
Notifications
You must be signed in to change notification settings - Fork 336
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
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
[Feature Request] Make WithCustomEmbeddingGenerator accepts type instead of concrete class #334
Comments
Analyzing the current implementation based on concrete types: kernel-memory/service/Abstractions/KernelMemoryBuilderExtensions.cs Lines 122 to 140 in 775301a
kernel-memory/service/Core/KernelMemoryBuilder.cs Lines 183 to 187 in 775301a
I see that the same instance is added as Singleton for the retrieval (line 132) and to a list of embedding generators for the ingestion phase (line 137). Then, in the library, classes receive in constructors the So, handling both scenarios with the same interface in Dependency Injection require a little refactoring, because we need a single I have made some investigations and I think we can handle this requirement using a Keyed Service for retrieval, while keeping using the classic registration for ingestion. In this way, we can get the retrieval I have created a little POC to showcase this scenario. @dluc, if you think this is a valuable feature, I can share the POC with you, before implementing the solution in Kernel Memory, so you can take a look at how I'm thinking to use keyed services. NOTE: I have talked about |
yes we need keyed services for LLMs for a bunch of scenarios, for embedding, text generation, chunking etc. Open to look at a PoC but it will take some time because I'd like the system also to support falling back to a default, and being easy to reuse across the project. |
In my extensions package I've used a similar structure of IHttpFactory, I'm registering all the needed interface as singleton, I can also used keyed registration. Then I register one or more UserQuestionPipeline factory, and I simply add handler as standard interfaces, then I can resolve the factory and let it creates the concrete implementation. It still needs some work but actually it works without too much problem |
this is just what I need to solve current issue I am having, I use LLAMa for chat completion but still want to use OpenAI for embedding; the problem is when I change the url to Llama server, it assume there is an embedding endpoint to use just like OpenAI. can someone approve the PR quick? also, the example of how to use this new feature would be helpful too |
@boreys you should already be able to do that. There are examples in the repo mixing Llama and OpenAI, also OpenAI and LM Studio, e.g.
|
IHttpClientFactory allows to link custom HttpClient instances "by key" and "by type" ("Typed Clients"). The typed client approach seems much better, e.g. not requiring global constants and not requiring to touch constructors, e.g. no need to reference IHttpClientFactory explicitly and no magic attributes. By contrast, the use of Hoping that will be addressed in future... if keyed services works only with string keys, I think we should at least avoid global constants, for instance using the class full name as a key. Question: how are services resolved if a keyed dependency is missing? The framework should fall back to a default implementation, does it? |
Actually, I have noticed that, if I use the In any case, once understood what it the correct behavior, if necessary it will be easy to provide a fall back to a default implementation. |
have you tried using var kmBuilder = new KernelMemoryBuilder();
// my service registrations
kmBuilder.Services.Add...
kmBuilder.Services.Add...
kmBuilder.Services.Add...
// use my service registrations to instantiate MyEmbeddingGeneratorClass
kmBuilder.Services.AddSingleton<ITextEmbeddingGenerator>(serviceProvider =>
{
return ActivatorUtilities.CreateInstance<MyEmbeddingGeneratorClass>(serviceProvider);
}); |
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
Context / Scenario
I want to use a custom EmbeddingGenerator that depends on some services that are registered in Dependency INjection.
The problem
When I use WithCustomEmbeddingGenerator to use a custom embedding generator I can use a class that depends on some other services, so I'd like to register like
.WithCustomEmbeddingGenerator
Actually I need to create a concrete class so I need to manually resolve all services.
Proposed solution
using .WithCustomEmbeddingGenerator allows me to register the class so when the KernelMemory needs to use my custom embedding generator it will resolve with DependencyInjection mechanism.
Importance
nice to have
The text was updated successfully, but these errors were encountered: