semantic-kernel icon indicating copy to clipboard operation
semantic-kernel copied to clipboard

Suggested tweaks to Embedding types

Open stephentoub opened this issue 2 years ago • 1 comments

Currently, Semantic Kernel contains:

// *** In Microsoft.SemanticKernel.dll ***
namespace Microsoft.SemanticKernel.AI.Embeddings
{
    public static class SupportedTypes
    {
        public static bool IsSupported<T>();
        public static bool IsSupported(Type type);
        public static IEnumerable<Type> Types { get; }
    }

    public ref struct EmbeddingSpan<TEmbedding>
    {
        public static bool IsSupported { get; }
        ...
    }

    public ref struct EmbeddingReadOnlySpan<TEmbedding>
    {
        public static bool IsSupported { get; }
        ...
    }
    ...
}

// *** In Microsoft.SemanticKernel.Abstractions ***
namespace Microsoft.SemanticKernel.AI.Embeddings
{
    public readonly struct Embedding<TEmbedding>
    {
        ...
        public static bool IsSupported { get; }
    }

    public static class Embedding
    {
        public static readonly Type[] SupportedTypes { get; }
    }
}

This is duplicative. The Embedding.SupportedTypes property is also returning an array that can be mutated without requiring any casting. I propose:

  • Remove the IsSupported properties from Embedding<T>, EmbeddedSpan<T>, and EmbeddedReadOnlySpan<T>.
  • Move the contents of the SupportedTypes class to the Embedding class, while also renaming the Types property to be SupportedTypes and deleting the one that's currently on Embedding.
  • Delete the SupportedTypes class.

That would result in:

// *** In Microsoft.SemanticKernel.dll ***
namespace Microsoft.SemanticKernel.AI.Embeddings
{
    public ref struct EmbeddingSpan<TEmbedding>
    {
        ...
    }

    public ref struct EmbeddingReadOnlySpan<TEmbedding>
    {
        ...
    }
    ...
}

// *** In Microsoft.SemanticKernel.Abstractions ***
namespace Microsoft.SemanticKernel.AI.Embeddings
{
    public readonly struct Embedding<TEmbedding>
    {
        ...
    }

    public static class Embedding
    {
        public static bool IsSupported<TEmbedding>();
        public static bool IsSupported(Type type);
        public static IEnumerable<Type> SupportedTypes { get; }
    }
}

cc: @dluc, @shawncal

stephentoub avatar Apr 21 '23 02:04 stephentoub

@awharrison-28

craigomatic avatar Apr 21 '23 17:04 craigomatic