Open wrzr123 opened 9 months ago
We should probably also update the embedding endpoint helper functions to add an optional model parameter (currently it's not one of the parameters because there's only been one model), as well as update the embedding request to support dimensions. But adding these models is a good start.
True! I only added what I needed, and the new models were enough for me, as the model can already be stated in the constructor of the EmbeddingRequest. I'm on holiday for the next week, but if you like I can have a look and enhance the helper functions and also the dimensions. I also thought about changing the default model to text-embedding-3-small, as the ada-model will probably become deprecated in a while. But this would be a problem with backwards compatibility for many users I guess.
Yeah, I think it’s probably best to leave the current default which is important for backwards compatibility. OpenAI has explicitly stated they won’t be deprecating that old model.
Perhaps I need to make this more clear on the documentation, but you can always provide a text string for the model name rather than one of the strongly typed models. So whenever OpenAI releases a new model, you can just pass in the model name without having the modify the SDK at all.
Anyway it’s a pretty hectic weekend for me, but I’ll do my best to double check all this and merge it early in the week.
-Roger
On Wed, Feb 7, 2024 at 2:13 PM wrzr123 @.***> wrote:
True! I only added what I needed, and the new models were enough for me, as the model can already be stated in the constructor of the EmbeddingRequest. I'm on holiday for the next week, but if you like I can have a look and enhance the helper functions and also the dimensions. I also thought about changing the default model to text-embedding-3-small, as the ada-model will probably become deprecated in a while. But this would be a problem with backwards compatibility for many users I guess.
— Reply to this email directly, view it on GitHub https://github.com/OkGoDoIt/OpenAI-API-dotnet/pull/193#issuecomment-1933024808, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJBNJCV4LQEDUP2ICUSNXDYSP4CFAVCNFSM6AAAAABC5TILSGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZTGAZDIOBQHA . You are receiving this because you commented.Message ID: @.***>
Alright, I made a new commit which includes
I've already found the option to initialize a custom model, was easy to find when looking at the code base. As I don't want to use customized libraries, I did it that way in my codebase which uses your nuget package. But of course I'm happy to update soon to the new version once available. ;)
One more question regarding running the tests. I've let the embeddings tests run locally, to make sure I didn't break anything. To make it work, I manually inserted my OpenAI API key in the constructors. And of course I forgot to take it out again before committing... I'm glad OpenAI and Github both notified me right away about the issue. Is there a better way to let the tests run locally? Or do you do it the same way and just always make sure to remove the key before publishing?
Added 2 new embedding models text-embedding-3-small and text-embedding-3-large