Support vector search on Cosmos DB#33991
Conversation
ajcvickers
force-pushed
the
VelocityIsAVector
branch
from
db246d8 to
d69a4d6
Compare
|
@roji I added a JSON fragment expression, as we discussed. |
roji
left a comment
roji
left a comment
There was a problem hiding this comment.
Looks pretty good! See mainly nits.
Probably a good idea for @AndriySvyryd to take a look for the metadata/model and CosmosClientWrapper stuff.
ajcvickers
force-pushed
the
VelocityIsAVector
branch
3 times, most recently
from
8c3fdf6 to
15f5a2c
Compare
|
@Pilchie What is the new guidance here given that the latest SDK has made all the code to configure vector indexes and embeddings internal? |
ajcvickers
force-pushed
the
VelocityIsAVector
branch
from
15f5a2c to
c43d3e8
Compare
|
This has been updated to the latest SDK and the container configuration has been made to throw. Verified that all the tests pass if the container has already been created. |
ajcvickers
force-pushed
the
VelocityIsAVector
branch
from
c43d3e8 to
91b866a
Compare
AndriySvyryd
left a comment
AndriySvyryd
left a comment
There was a problem hiding this comment.
Add a test to CompiledModelCosmosTest
ajcvickers
force-pushed
the
VelocityIsAVector
branch
4 times, most recently
from
59ce915 to
b2b21ae
Compare
|
@roji This is now constrained. |
roji
left a comment
roji
left a comment
There was a problem hiding this comment.
LGTM.
As discussed offline, we really should deliver this feature with support for arrays as well (e.g. float[]). Maybe the best way forward is to build in support for it in this PR for the type mapping side and have failing tests for the query translation, which I'd work on fixing after rc1 (given feature work stops in a few days).
BTW at what level is Half (float16) not supported? Is the Cosmos SDK the blocking one here? If the intention is for it to be supported soon, it may be worth just implementing it on our side and letting Cosmos/the SDK fail, since it'll be harder to add it in later.
| /// <param name="property">The property.</param> | ||
| /// <param name="vectorType">The type of vector stored in the property.</param> | ||
| [Experimental(EFDiagnostics.CosmosVectorSearchExperimental)] | ||
| public static void SetVectorType(this IMutableProperty property, CosmosVectorType? vectorType) |
There was a problem hiding this comment.
BTW does Cosmos actually support nullable vector properties? Might be worth checking
| <value>The type '{givenType}' cannot be mapped as a dictionary because it does not implement '{dictionaryType}'.</value> | ||
| </data> | ||
| <data name="BadVectorDataType" xml:space="preserve"> | ||
| <value>The type '{clrType}' is being used as a vector, but the vector data type cannot be inferred. Only 'ReadOnlyMemory<byte>, ReadOnlyMemory<sbyte>, and ReadOnlyMemory<float> are supported.</value> |
There was a problem hiding this comment.
As discussed offline, arrays should also work.
ajcvickers
force-pushed
the
VelocityIsAVector
branch
from
b2b21ae to
8efd22d
Compare
7 participants
Fixes #33783
This PR introduces:
IsVector()to configure a property to be configured as a vector (embedding) in the document.HasIndex().ForVectors()to configure a vector index over a vector property.VectorDistance()which translates to the CosmosVectorDistancefunctionKnown issues: