-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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: Bump ONNX to 0.5.2 #9644
.Net: Bump ONNX to 0.5.2 #9644
Conversation
Doesn't this change mean that the model is reloaded into memory every time that you run an inference? This might take several seconds each time.
|
@f2bo This won't be merged as is, I will add some extra Unit Testing to ensure it doesn't affect the status-quo, for the moment recommend using the |
…nto issues/9628-adding-ogahandle-resource
Rather than relying on users to ensure that an OgaHandle is created and disposed, I wonder if you have any objections to the following change: public sealed class OnnxRuntimeGenAIChatCompletionService : IChatCompletionService, IDisposable
{
...
private readonly static OgaHandle s_ogaHandle = new();
static OnnxRuntimeGenAIChatCompletionService()
=> AppDomain.CurrentDomain.ProcessExit += (_, _) => s_ogaHandle.Dispose();
...
} |
@f2bo Yes, I personally object this approach as this injects static resources and behavior on customers environment/application domain without their knowledge to handle a non-conventional resource monitoring. And some other considerations regarding the potential pitfalls of using
|
Co-authored-by: westey <164392973+westey-m@users.noreply.github.com>
Motivation and Context
The Latest 0.5.0 package also requires the caller to handle the resources with the
OgaHandler
instance, when a service is instantiated this resource needs to be also present and exposed together with the service. Otherwise a message will be sent to the console and the application will crash before finishing.Resolves .Net: Bug: Application crashes when using the ONNX connector and OnnxRuntimeGenAI v0.5.0 package #9628