search

elastic / eland

Python Client and Toolkit for DataFrames, Big Data, Machine Learning and ETL in Elasticsearch
https://eland.readthedocs.io
Apache License 2.0
635 stars 98 forks source link

Simplify embedding model support and loading #569

Closed joshdevins closed 1 year ago

joshdevins commented 1 year ago

We were attempting to load SentenceTransformers by looking at the model prefix, however SentenceTransformers can also be loaded from other orgs in the model hub, as well as from local disk. This prefix checking failed in those two cases. To simplify the loading logic and deciding which wrapper to use, we’ve removed support for text_embedding tasks to load a plain Transformer. We now only support DPR embedding models and SentenceTransformer embedding models. If you try to load a plain Transformer model, it will be loaded by SentenceTransformers and a mean pooling layer will automatically be added by the SentenceTransformer library. Since we no longer automatically support non-DPR and non-SentenceTransformers, we should include somewhere example code for how to load a custom model without DPR or SentenceTransformers.

Note: This change will allow us to support E5 embeddings uploaded with eland. This change will not yet solve adding the preamble instructions to query and index embeddings.

See: https://github.com/UKPLab/sentence-transformers/blob/v2.2.2/sentence_transformers/SentenceTransformer.py#L801

Resolves #531

joshdevins commented 1 year ago

Blog post to follow, draft WIP

davidkyle commented 1 year ago

@picandocodigo the old Jenkins ci is still present in Eland PRs, you mentioned last week that it will go away once the infra repository is updated. Is there any progress?

https://github.com/elastic/eland/pull/561#issuecomment-1640079677

The good news is that buildkite is working

joshdevins commented 1 year ago

@picandocodigo I've also rebased after you added that "ignore" file, and I've tried opening a new PR but I get the same build error.

picandocodigo commented 1 year ago

@davidkyle @joshdevins great that Buildkite is working now! I removed clients-ci from the requested checks in the branch protection rules, so it should be possible to merge without that passing. But there's some work still pending on infrastructure for this to be fully removed, I'll update you soon!

serenachou commented 1 year ago

related PR: https://github.com/elastic/enterprise-search-pubs/pull/3666