.Net: Promote the new Oobabooga repo/package (and remove from SK)

[stephentoub]

Important

Labeled Urgent because it may require a breaking change if we should remove TextCompletionRequest.

There are two types that derive from AIRequestSettings: OpenAIRequestSettings and TextCompletionRequest. The latter is specific to the Oobabooga connector but doesn't have Oobabooga in the name nor the RequestSettings suffix. It's not clear to me how someone is supposed to know they should be creating a TextCompletionRequest in order to fulfill an AIRequestSettings parameter.

To solve this, we will remove it from our current repo and point users to the new Oobabooga repo/package.

[dmytrostruk]

Agreed, TextCompletionRequest from Oobabooga package should be renamed to OobaboogaRequestSettings.

[matthewbolanos]

Based on @dmytrostruk, the Oobabooga connector now has its own repo. We should mark all of the existing Oobabooga functionality deprecated and redirect to the new repo.

Will change name/description to reflect the change.

[jsboige]

Adding my 2 cents here since I'm currently trying to update the new Oobabooga connector, the latest Nuget package of which seems to have issues that didn't appear upon initial tests and were recently reported.

I went ahead updating the reference to the new Beta Nuget package, and now I'm scratching my head trying to adapt to the new AIRequestSettings model, not sure to understand the way forward, especially for the MultiConnector.

I mean I've read about the 3 evolution options considered and the final choice but the current implementation has me puzzled by the way the new OpenAI settings we implemented.

I understand the new ExtensionData dictionary is supposed to be a way to share properties like temperature or max tokens between prompt settings and various connectors, in a more flexible way that the former rigid structure but the OpenAI implementation only seems to use it one way through this weird json serialization/deserialization with a custom converter.

Accordingly, that seems to mean to me there won't be any simple way to serialize/deserialize AIRequestSettings in a general manner, nor to recover the actual settings apart from when the new ExtensionData property is used, which does not seem to be the case when OpenAI request settings are created.

This is going to make updating the Multiconnector pretty hard, and I'm a bit lost on what to do with Oobabooga.

Shouldn't all concrete implementation of AIRequestSettings have ensured that dictionary property was used in the first place?

[dmytrostruk]

@jsboige Thanks for reporting, I would like to understand your use-case better, if possible.

In general, you should be able to define class OobaboogaRequestSettings : AIRequestSettings and inside this class define properties with Oobabooga-related settings. Then, in Oobabooga connector you should be able to check if requestSettings is OobaboogaRequestSettings oobaboogaRequestSettings and then work with your settings instance. Serialization/deserialization is not required at this point.

but the OpenAI implementation only seems to use it one way through this weird json serialization/deserialization with a custom converter

Custom converter was implemented with purposes to support different naming cases for settings in config.json file. For example, there won't be any difference if user will have max_tokens or MaxTokens. But it's optional feature, and it is not required for Oobabooga or any other connectors.

This is going to make updating the Multiconnector pretty hard, and I'm a bit lost on what to do with Oobabooga.

It would be really helpful, if you could share some code examples where AIRequestSettings class doesn't cover your case, so we will be able to understand how we can improve it.

Shouldn't all concrete implementation of AIRequestSettings have ensured that dictionary property was used in the first place?

So, dictionary property was introduced as additional feature for flexibility. You can use only this property and access your settings in connector like requestSettings["MyProperty"]. But if you want to work with settings using strong types like requestSettings.MyProperty, then you can define class OobaboogaRequestSettings: AIRequestSettings and use it like:

var function = kernel.CreateSemanticFunction(prompt, requestSettings: new OobaboogaRequestSettings { MyProperty = "value" });

Please, let me know if this information is helpful! Thank you!

[jsboige]

@dmytrostruk thanks for your answer.
This is in line with what I understood, and I started implementing the new Oobabooga text and chat completion request settings the way the OpenAI request settings were implemented (that is, keeping the concrete properties that preexisted to the change).
I also started adding a similar method to that of OpenAI connector trying to make sense of the AIRequestSettings parameter, figuring out if it's a native Oobabooga settings instance, or one of a distinct kind.

I find it a bit cumbersome that we don't know really what's in this dictionary depending on the original AIRequestSettings, whether actual "soft" properties manually inserted at some point, JsonElements, as a result of the JsonExtensionData attribute upon deserialization of prompt settings, or nothing at all if settings are of a concrete type that do not make use of them.
The fact that the OpenAI implementation relies on serializing and deserializing using a custom converter low-level primitives to make sense of it is a bit frightening and hard to grasp.

As for the multiconnector, it is supposed to work seemlessly with request settings regardless of their kinds, with several stages where they can be passed over unchanged or updated depending on the model-specific configurations, and there are several places where custom settings are serialized or deserialized.

I will have to think about how to upgrade that...

Anyway, I believe I have the global picture, and will try my best with the current state.

[dmytrostruk]

I find it a bit cumbersome that we don't know really what's in this dictionary depending on the original AIRequestSettings, whether actual "soft" properties manually inserted at some point, JsonElements, as a result of the JsonExtensionData attribute upon deserialization of prompt settings, or nothing at all if settings are of a concrete type that do not make use of them.

@jsboige As Connector developer, you define how you want to work with settings, and at the point when you receive AIRequestSettings, as author of Oobabooga connector, you should already know how to handle it in your Connector.

For example, if you add new Connector and you say that it should work with MySettings : AIRequestSettings, then users of your connector should pass MySettings when they create semantic function, and they should follow the same naming in their config.json.

But if you say that there is no concrete type of settings and users should set them in ExtensionData, then users will have to create new instance of AIRequestSettings and set all properties in dictionary, and then in your Connector you will be able to receive it. It's also an option, but from user perspective, there will be no strong types for settings, which makes it harder to control them in compile-time.

If users pass some settings, which are not compatible with Oobabooga settings pattern, then you should probably throw an exception in Connector, or try to use default settings, if it's acceptable scenario.

Anyway, I believe I have the global picture, and will try my best with the current state.

Please let us know if any help is needed. Your use-case is very helpful, because it will allow us to understand if current AIRequestSettings implementation is scalable enough, or something should be improved. Thanks a lot!

[jsboige]

For example, if you add new Connector and you say that it should work with MySettings : AIRequestSettings, then users of your connector should pass MySettings when they create semantic function, and they should follow the same naming in their config.json.

Sure, dealing with the concrete Oobabooga settings should be straightforward and the documented regular case. It's the other cases that I find difficult. Again, having to serialize and deserialize using a custom converter because you don't really know what's in the object but you expect it might have shared parameters like temperature or maxtokens looks like a strange evolution from the base class where those properties were well defined and you didn't have to guess them from the json result.

For Oobabooga, I might try to use the dictionary directly to save on perfs because we're not expecting something different the JsonElements deserialized from config.json. But for the multiconnector, I guess I'm left with the same OpenAI approach of serializing / deserializing and hope for the best.

[dmytrostruk]

but you expect it might have shared parameters like temperature or maxtokens looks like a strange evolution from the base class where those properties were well defined and you didn't have to guess them from the json result.

Parameters like temperature or maxtokens were defined initially based on OpenAI API, but they are not common across different connectors (e.g. Hugging Face). But if you have similar parameters in Oobabooga, you can define OobaboogaRequestSettings and define same parameters there. It should be possible to avoid serialization/deserialization completely and use just strong type for settings.

[jsboige]

Sure, that's the plan, thanks

[jsboige]

@dmytrostruk I just published a new Nuget package, with an attempt at following the new request settings format.
I'm not super satisfied with the current state, but it should do for a while since there were issues that needed fixing.

Oobabooga RequestSettings (this class and that one) are relatively similar to OpenAI, in that they will attempt to read the extended properties only if the passed in settings are not of the concrete type or OpenAI's. They also account for the fact that MaxTokens has a different name in Oobabooga (MaxNewTokens), and that RepetitionPenalty has a different scale (that was already implemented in the previous version). Also, they don't rely on serialization and will directly read JsonElement values or use IConvertible if the dictionary contains primitive types instead.

As for the MultiConnector, I introduced specific settings class that tries to keep everything in the extended properties' dictionary, while upper casing keys.
When some parameters such as temperature or max tokens are intercepted, the values are changed from JsonElements to primitive types.
The intent is that input settings could be any settings from OpenAI, Oobabooga or Prompt template config, MultiConnector settings then import and flatten everything inside the dictionary in a common implementation, optionally performs updates to maxtoken or temperature during the pipeline, and then either OpenAI or Oobabooga should be able to recover the settings at the other end of the pipeline.

That was tested on the existing simple examples, but in order to support all cases, I will need to rework those settings to account for and map all existing connectors specific settings properties.
This is inline with what I had foreseen, thus not very convenient. To be honest, I believe the Oobabooga way of having concrete settings with all properties for all supported models was a bit dumb but so much easier, since most models share things like temperature, maxtokens etc., and here we're now left with trying to guess their names and dealing with the hybrid serialization hack to account for concrete properties on top of the dictionary.

Anyway, I guess that makes up for some feedback for your team, and I'll be happy to know what you think is the best way forward from there.

[dmytrostruk]

Oobabooga RequestSettings (this class and that one) are relatively similar to OpenAI, in that they will attempt to read the extended properties only if the passed in settings are not of the concrete type or OpenAI's.

@jsboige I would suggest removing OpenAI dependency from Oobabooga Connector. Even if settings for both Connectors are similar or exactly the same, it's still separate Connectors and having different types for them (even with the same properties) is completely fine.

Here is the example, how the settings could be defined for semantic function:

{
  "schema": 1,
  "description": "My semantic function",
  "models": [
    {
      "service_id": "open-ai-gpt-4",
      "max_tokens": 150,
      "temperature": 0.9,
      "top_p": 0.0
    },
    {
      "service_id": "oobabooga-ai-model",
      "max_new_tokens": 200,
      "repetition_penalty": 1.18,
      "do_sample": false
    },
  ]
}

At the point of semantic function execution, it should recognize available AI Connectors in Kernel and map settings to specific Connector. Also, it should be possible to specify settings not only on Connector level, but on AI Model level (e.g. For OpenAI, there will be separate settings for gpt-4, gpt-3.5-turbo etc.) If settings for different AI Models from the same AI provider (e.g. OpenAI) are the same, they can share the same settings type. But if it's different AI providers (OpenAI, Oobabooga, HuggingFace), it's probably better to have different types, because you won't be coupled to another Connector when you want to add/remove/update some property in the future.

They also account for the fact that MaxTokens has a different name in Oobabooga (MaxNewTokens), and that RepetitionPenalty has a different scale

That's exactly the reason why we shouldn't try to align settings to the same type/format. They are just different by nature and should be implemented separately.

To be honest, I believe the Oobabooga way of having concrete settings with all properties for all supported models was a bit dumb but so much easier, since most models share things like temperature, maxtokens etc.

From Semantic Kernel perspective, we should be compatible with different AI providers, AI models and related settings for them. In Hugging Face, there are around 30,000 text completion models available at the moment and most probably the settings for each of them are different as well.

and here we're now left with trying to guess their names and dealing with the hybrid serialization hack to account for concrete properties on top of the dictionary.

In your scenario, I would suggest avoiding dictionary property at all (at least in first implementation). Try to use just concrete type (OobaboogaCompletionRequestSettings) and rely on snake_case in your settings (e.g. max_new_tokens). If you remove OpenAI dependency and dictionary deserialization logic, your Connector and settings should still be compatible with SK. If it's not the case, please let us know and we will add required changes to support that scenario.

I hope that makes sense and thank you for your feedback! I would be happy to continue this conversation and see if current setting model is scalable enough to cover your and other cases. Thanks again!

[jsboige]

@jsboige I would suggest removing OpenAI dependency from Oobabooga Connector. Even if settings for both Connectors are similar or exactly the same, it's still separate Connectors and having different types for them (even with the same properties) is completely fine.

@dmytrostruk I didn't plan to support OpenAI in the Oobabooga connector to start with, and you're probably right it is overkill. I just realized the OpenAI dependency already exists in the regular Semantic-kernel nuget package so I thought why not. But this is a very edge case indeed. Maybe I should reference a more restricted package?

In your scenario, I would suggest avoiding dictionary property at all (at least in first implementation). Try to use just concrete type (OobaboogaCompletionRequestSettings) and rely on snake_case in your settings (e.g. max_new_tokens).

As for the dictionary property, it is necessary to support it as it is where the prompt template config params reside, in the form of deserialized JsonElement properties. OpenAI connector does it through a round trip of json serialization/deserialization and a custom json converter. I chose to tap in the dictionary property directly to avoid the overhead, but I believe it is pretty much equivalent.

Now the multiconnector is a different story. It has to harmonize accross models and prompt settings by definition, on the way in and on the way out, thus the use of the same serialization mechanism to flatten both concrete and dictionary properties.

As for Huggingface, this is pretty much what Oobabooga is about. Pretty much all the models in the open LLM leaderboard are supported in Oobabooga, and I can tell you that apart from specific properties like mirostat for Llama.cpp models, pretty much all of them use the set of common properties that the previous ChatGPT inspired shared Completion settings exhibited. Only the scale for repetition penalty and beam search instead of alternate responses were significantly different.

Anyway, I'll remove the OpenAI dependency on next release, but for now, it seems to work fine. Let's see what users say.

[dmytrostruk]

I just realized the OpenAI dependency already exists in the regular Semantic-kernel nuget package so I thought why not. But this is a very edge case indeed. Maybe I should reference a more restricted package?

@jsboige So, if we look from SK user perspective, if I want to use SK in my applications, I install SK and I have OpenAI Connector out of the box in order to be able to quickly jump in and integrate with AI. But if I'm SK Plugin or Connector developer, in ideal scenario I need to use just SemanticKernel.Abstractions package in order to provide my own implementation of some Plugin or Connector, so it can be re-usable by others. At the moment we are testing if it's possible to develop using SemanticKernel.Abstractions only for Plugin and Connector authors. If you have a chance to see, if Oobabooga Connector can be implemented with provided abstractions only, and share your feedback with us, that would be awesome!

As for the dictionary property, it is necessary to support it as it is where the prompt template config params reside, in the form of deserialized JsonElement properties. OpenAI connector does it through a round trip of json serialization/deserialization and a custom json converter. I chose to tap in the dictionary property directly to avoid the overhead, but I believe it is pretty much equivalent.

I got the point now. Yes, so in current implementation you can work with strong settings type if you inject it programmatically, but when you load them from config.json file you will receive it in ExtensionData:

// Pass settings instance directly - access MySettings in Connector
var semanticFunction1 = kernel.CreateSemanticFunction("template", requestSettings: new MySettings { Text = "text", Number = 2 });

// Initialize settings instance with config.json - access ExtensionData in Connector
var semanticFunction2 = kernel.CreateSemanticFunction("template", PromptTemplateConfig.FromJson(GetConfigPayload()));

But even with this case, ExtensionData deserialization should be simple. You don't need to implement switch/case to handle each property separately. This is something we consider removing in the future. Basically, you can have your FromRequestSettings method very similar to OpenAI one, but without JsonConverter logic:

semantic-kernel/dotnet/src/Connectors/Connectors.AI.OpenAI/OpenAIRequestSettings.cs

Lines 131 to 155 in 18ffc4b

	public static OpenAIRequestSettings FromRequestSettings(AIRequestSettings? requestSettings, int? defaultMaxTokens = null)
	{
	if (requestSettings is null)
	{
	return new OpenAIRequestSettings()
	{
	MaxTokens = defaultMaxTokens
	};
	}
	if (requestSettings is OpenAIRequestSettings requestSettingsOpenAIRequestSettings)
	{
	return requestSettingsOpenAIRequestSettings;
	}
	var json = JsonSerializer.Serialize(requestSettings);
	var openAIRequestSettings = JsonSerializer.Deserialize<OpenAIRequestSettings>(json, s_options);
	if (openAIRequestSettings is not null)
	{
	return openAIRequestSettings;
	}
	throw new ArgumentException($"Invalid request settings, cannot convert to {nameof(OpenAIRequestSettings)}", nameof(requestSettings));
	}

Your FromRequestSettings method will have three phases:

1. Check if settings are null, and if yes - provide default one.
2. Try to cast it to strong type and if it's successful - use it.
3. Deserialize and use it.

Please let me know if this scenario will work for you. Thank you!