feat: Embed modes and named vectors

[pwalski]

Added named vector support to qdrant. A pipeline can now have its embed mode configured, either per field, chunk and metadata combined (default) or both. Vectors need to be configured on the qdrant client side.

See examples/store_multiple_vectors.rs for an example.

Shoutout to @pwalski for the contribution. Closes #62.

[timonv]

Look good already, nice! Maybe there's a way to clean up the qdrant configuration. If you always need an EmbeddableType and sometimes a Vector config, you could also have the function signature like:

    pub fn with_vector(
        mut self,
        vector: impl Into<VectorConfig>,
    ) 

And then implent Into<VectorConfig> for embedabletype.

One requirement I do have is that if needed, qdrant can be utilized fully. What happens if a vector config is provided, but a custom client is build?

What I really like about your implementation is that it also sets swiftide up to infer the schema from the pipeline at a later stage.

[pwalski]

One requirement I do have is that if needed, qdrant can be utilized fully. What happens if a vector config is provided, but a custom client is build?

I do not understand. Qdrant client implementation from this library limits user's ability to configure Vectors by providing only few config options in VectorConfigBuilder (name, size, distance). New Qdrant client implementation would need to come with new implementation of configuring Vectors.
The same goes with other Qdrant config options. QdrantBuilder provides only a subset of qdrant_client::Qdrant config options.

[timonv]

One requirement I do have is that if needed, qdrant can be utilized fully. What happens if a vector config is provided, but a custom client is build?

I do not understand. Qdrant client implementation from this library limits user's ability to configure Vectors by providing only few config options in VectorConfigBuilder (name, size, distance). New Qdrant client implementation would need to come with new implementation of configuring Vectors. The same goes with other Qdrant config options. QdrantBuilder provides only a subset of qdrant_client::Qdrant config options.

Exactly, I don't see the need to replace / reinvent full apis from other libraries that already work. Most (if not all) integrations support setting whatever it wraps manually, allowing for more fine grained configuration. So we provide some basic stuff for sensible defaults, otherwise it's RTM on the integrations' documentation.

I thought the current setup might go wrong when overriding the client and accidentally also providing vector configuration.

[pwalski]

@timonv I cannot assign a reviewer.

[timonv]

Solid work! I have a couple of comments and some suggestions.

I think EmbeddedType could be more apptly named as EmbeddedField or EmbedField, as that seems to be more what it is.

Maybe there's also a way to clean up the api for .with_vector, .with_vector(EmbeddableType::Metadata(metadata_summary::NAME.into())), do you have any ideas?

On a more practical note, this looks very close to merging and I would love to get it in for 0.6! Do you have any social handles I can mention on release? 🎉

[pwalski]

Do you have any social handles I can mention on release?

I do not have any actively updated profiles.

Maybe there's also a way to clean up the api for .with_vector, .with_vector(EmbeddableType::Metadata(metadata_summary::NAME.into())), do you have any ideas?

These NAME fields got created because I had no idea how to configure Metadata and share info about metadata/embedding name with vector config.
The fact user needs to specify same Metadata once as a Transformer and once as a VectorConfig name is already a bit awkward.

The quoted line could be cleaned up a bit by providing following impl for EmbeddedField:

impl From<&str> for EmbeddedField {
    fn from(value: &str) -> Self {
        Self::Metadata(value.into())
    }
}

The problem is I am not sure how long such From impl would hold. It could become a problem even during implementation of Sparse vectors #63.
That task will introduce another usecase for named vectors so it could be a good opportunity to make some updates to vectors config.

Making EmbeddedField generic over some T: Into<String> + Display would cause propagation of generic type across codebase.
Making EmbeddedField::Metadata accept &str would require usage of a lifetime, which creates same problem as generic.

[timonv]

Do you have any social handles I can mention on release?

I do not have any actively updated profiles.

Maybe there's also a way to clean up the api for .with_vector, .with_vector(EmbeddableType::Metadata(metadata_summary::NAME.into())), do you have any ideas?

These NAME fields got created because I had no idea how to configure Metadata and share info about metadata/embedding name with vector config. The fact user needs to specify same Metadata once as a Transformer and once as a VectorConfig name is already a bit awkward.

The quoted line could be cleaned up a bit by providing following impl for EmbeddedField:

impl From<&str> for EmbeddedField {
    fn from(value: &str) -> Self {
        Self::Metadata(value.into())
    }
}

The problem is I am not sure how long such From impl would hold. It could become a problem even during implementation of Sparse vectors #63. That task will introduce another usecase for named vectors so it could be a good opportunity to make some updates to vectors config.

Making EmbeddedField generic over some T: Into<String> + Display would cause propagation of generic type across codebase. Making EmbeddedField::Metadata accept &str would require usage of a lifetime, which creates same problem as generic.

Fully agree, I don't see a direct option here either. I have hunch that when we get further down the line, a nicer model will pop up. I.e. a pipeline could infer a schema.

Feel it's ready to merge? 0.6 will probably be released this week. Need to update and rewrite a bunch of documentation on swiftide.rs for it, it was setup a bit in a hurry.

[pwalski]

Feel it's ready to merge?

Yes.
There are issues like the fact with_embed_mode() needs to be placed before then(impl Transformer), and multiple then_store_with() do not work, but it is not related directly to this PR.

[timonv]

Multiple then store with don’t work? That’s a bug, could you open an issue? Sent from [Proton Mail](https://proton.me/mail/home) for iOS

…

On Tue, Jul 9, 2024 at 14:42, pwalski ***@***.***(mailto:On Tue, Jul 9, 2024 at 14:42, pwalski <<a href=)> wrote: > Feel it's ready to merge? Yes. There are issues like the fact with_embed_mode() needs to be placed before then(impl Transformer), and multiple then_store_with() do not work, but it is not related directly to this PR. — Reply to this email directly, [view it on GitHub](#123 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AAAMBXLJFNPG4JLLYZMO2S3ZLPLDBAVCNFSM6AAAAABKJUQL5CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMJXGYYDAOJUGY). You are receiving this because you were mentioned.Message ID: ***@***.***>

[pwalski]

That’s a bug, could you open an issue?

The issue I remembered was really only a problem with ignoring some of the EmbeddedFields.
I implemented named vectors with an assumption that we want to store in Qdrant every created embedding, but internal code did not really blocked me from implementing handling skipping some of embeddings in case they were not configured in Qdrant as vectors.
I reported it as a bug and I have opened a PR
#138