Closes #112, adds VectorOptionsAttribute, #125
Closes #112, adds VectorOptionsAttribute, #125a-random-steve wants to merge 3 commits intodatastax:mainfrom
Conversation
…sAttribute to class level (not property)
| public string SourceModel { get; set; } | ||
|
|
||
| /// <summary>The name of the embedding service provider.</summary> | ||
| public string Provider { get; set; } | ||
|
|
||
| /// <summary>The model name to use for embedding generation.</summary> | ||
| public string ModelName { get; set; } |
There was a problem hiding this comment.
Are ModelName and SourceModel redundant? e.g. looking at python, I only see model_name: https://docs.datastax.com/en/astra-api-docs/_attachments/python-client/astrapy/info.html#astrapy.info.CollectionVectorOptions
There was a problem hiding this comment.
There was a problem hiding this comment.
Caution, these two should not be at the same level -- actually, assuming the shared goal of isomorphism with the Data API payloads, modelName should be within a "service" subobject together with provider (and authentication and/or parameters when they apply).
Here is a real payload for a createCollection with all the relevant stuff, for reference:
{
"createCollection": {
"name": "c",
"options": {
"vector": {
"dimension": 1024,
"metric": "dot_product",
"service": {
"provider": "nvidia",
"modelName": "nvidia/nv-embedqa-e5-v5"
},
"sourceModel": "bert"
}
}
}
}
There was a problem hiding this comment.
@sl-at-ibm Agreed, they are at different levels.
| public class VectorizeOptionsAttribute : Attribute | ||
| { | ||
| /// <summary>The number of dimensions for the vector.</summary> | ||
| public int Dimension { get; set; } = -1; |
There was a problem hiding this comment.
Is there a reason for Dimension and Metric to be here? (as opposed to being only 1-level-higher, in VectorOptionsAttribute where they belong)
On first sight, these should be removed from this class (and some of the definition-filling work in CollectionDefinition slightly revised accordingly).
5 participants
Also moves LexicalOptionsAttribute to class level (not property)
Fixes #112