Skip to content
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

Adopt Embedding abstraction from M.E.AI #10126

Open
markwallace-microsoft opened this issue Jan 8, 2025 · 4 comments
Open

Adopt Embedding abstraction from M.E.AI #10126

markwallace-microsoft opened this issue Jan 8, 2025 · 4 comments
Labels
Build Features planned for next Build conference memory connector memory

Comments

@markwallace-microsoft
Copy link
Member

No description provided.

@markwallace-microsoft markwallace-microsoft moved this to Backlog: Planned in Semantic Kernel Jan 8, 2025
@markwallace-microsoft markwallace-microsoft added triage memory connector memory Build Features planned for next Build conference and removed triage labels Jan 8, 2025
@roji
Copy link
Member

roji commented Jan 10, 2025

Some thoughts/questions:

  • Should new API(s) accepting Embedding be added alongside the existing ones (which accept arbitrary Ts), or should they replace them?
    • Replacing would mean a breaking change for existing users (which I believe shouldn't be a problem at this stage). If we do this, we need to do it before the API stabilizes.
    • Having only Embedding-based APIs is good for overload resolution; the current arbitrary T creates ambiguity e.g. between the the API accepting an embedding (on IVectorizedSearch) and the one accepting text to be vectorized internally (on IVectorizableTextSearch). The Embedding type neatly solves that problem.
    • Depending on where embeddings are coming from/how they're generated, it may be a bit onerous for users to provide an Embedding; if they e.g. load data from somewhere, they already have that available as a ReadOnlyMemory, and would then need to (needlessly) wrap it in an Embedding.
    • On the other hand, if typical usage will be to obtain an Embedding from an IEmbeddingGenerator, this simplifies the user experience (obviates having to extract the ROM out).
  • We need to better understand the intended usage of Embedding.AdditionalPropertiesDictionary - does that represent the metadata that's meant to be stored in the vector database and e.g. filtered on? If so, how would that interact with the current MEVD mechanism of managing metadata... /cc @stephentoub @SteveSandersonMS
  • We need to understand how sparse vectors fit into this (at least some vector DBs support sparse vectors, e.g. Milvus).
    • The current Embedding abstraction (code) is tied to ReadOnlyMemory<T>, and so cannot sparseness.
    • I'm wondering if Embedding<T> or a related type (e.g. SparseEmbedding<T> extending non-generic Embedding) should eventually be able to represent a sparse embedding. In addition, some sort of sparse vector .NET exchange type is probably needed (which would be wrapped by SparseEmbedding<T>.
    • /cc @stephentoub @SteveSandersonMS (do we have an issue/item tracking these things?)

@stephentoub
Copy link
Member

The current Embedding abstraction (code) is tied to ReadOnlyMemory, and so cannot sparseness.

Just to clarify, Embedding<T> is currently tied to ReadOnlyMemory<T>, but the base non-generic Embedding is not.

e.g. SparseEmbedding extending non-generic Embedding

Yeah, my expectation is we'd have another Embedding-derived type to represent a sparse embedding, assuming we have a good representation for one.

We need to better understand the intended usage of Embedding.AdditionalPropertiesDictionary

We don't currently have any use of it, but we added it because we have such a dictionary on many of the other types in the M.E.AI object model in order to facilitate implementations passing along whatever arbitrary additional information they have that might be associated with the embedding. We could remove it for now if it's problematic.

do we have an issue/item tracking these things?

Not to my knowledge.

@SteveSandersonMS
Copy link

We need to better understand the intended usage of Embedding.AdditionalPropertiesDictionary - does that represent the metadata that's meant to be stored in the vector database and e.g. filtered on?

No, definitely not. If we gave it that meaning, it would cease to be a loosely-typed bag of provider-specific stuff, and would become a first class API (which would awkwardly overlap with the "bag of provider-specific stuff" use case).

@roji
Copy link
Member

roji commented Jan 10, 2025

No, definitely not. If we gave it that meaning, it would cease to be a loosely-typed bag of provider-specific stuff, and would become a first class API (which would awkwardly overlap with the "bag of provider-specific stuff" use case).

Right. Yeah, at the very least we'll need to disambiguate this a little bit naming-wise - I can absolutely see users getting confused and thinking this is metadata to go into the vector DB, as opposed to arbitrary stuff produced by the embedding generator (which is what it is, right?). Do we have specific scenarios on how this bag will get used?

Since I'm not sure the vector database actually cares about anything inside Embedding other than the ROM (I think), maybe having it accept an Embedding is more trouble than it's worth... I do love it for the strong typing/overload disambiguation, and for simplifying usage when an Embedding is generated via IEmbeddingGenerator; but still...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build Features planned for next Build conference memory connector memory
Projects
Status: Backlog: Planned
Development

No branches or pull requests

4 participants