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

feat: to support similarity with relevance scores #12

Closed
wants to merge 1 commit into from

Conversation

cheriL
Copy link

@cheriL cheriL commented Oct 8, 2024

Description: to support _similarity_search_with_relevance_scores func in base class VectorStore with milvus.

@cheriL cheriL force-pushed the feat/relevance_scores branch from c15cf64 to da9bf05 Compare October 18, 2024 09:51
@cheriL
Copy link
Author

cheriL commented Oct 18, 2024

fix conflicts and lints.

@cheriL
Copy link
Author

cheriL commented Nov 6, 2024

From the CI error report, I just realized that the latest code supports multiple searches(I used to use the deprecated one...). So the problem may be more complicated than I thought, and the methods in the base class may not be available.

@zc277584121
Copy link
Collaborator

@cheriL thank you for your contribution. can you tell me what 's the issue without this contribution.
As i know ,there seems raise an issue in this code with current langchain_milvus code version:

retriever = vdb.as_retriever(
    search_type='similarity_score_threshold',
    search_kwargs={
        'score_threshold': 0.3,
        'k': 3,
    },
)
res = retriever.invoke("hello world")
print(res)
...
  File "/Users/zilliz/miniforge3/envs/py310/lib/python3.10/site-packages/langchain_core/vectorstores/base.py", line 560, in similarity_search_with_relevance_scores
    docs_and_similarities = self._similarity_search_with_relevance_scores(
  File "/Users/zilliz/miniforge3/envs/py310/lib/python3.10/site-packages/langchain_core/vectorstores/base.py", line 507, in _similarity_search_with_relevance_scores
    relevance_score_fn = self._select_relevance_score_fn()
  File "/Users/zilliz/miniforge3/envs/py310/lib/python3.10/site-packages/langchain_core/vectorstores/base.py", line 448, in _select_relevance_score_fn
    raise NotImplementedError

So I think maybe your contribution want to solve this sort of issue.
can you tell the origin purpose of this contribution. Just want to collect more cases and avoid all the potential issues

@cheriL
Copy link
Author

cheriL commented Nov 7, 2024

@zc277584121 My problem is similar to your example.
I'm working on building a RAG application using milvus as my vector store. when I expect to get docs and normalized scores by calling _similarity_search_with_relevance_scores, NotImplemented error raised. and I handle it by metric_type outside the function currently.
may also be something wrong with my usage?

@zc277584121
Copy link
Collaborator

@cheriL Currently the _similarity_search_with_relevance_scores() and _select_relevance_score_fn() are missing in Milvus class , which causes our issues, I will add them within about one day.
I'm now drafting the code by refering to this PR, but I found there are some potential problem by directly use _euclidean_relevance_score_fn when 'L2' and use _max_inner_product_relevance_score_fn when IP because the formulas in both methods can not gaurantee the so called relevance score are between [0, 1] . So I decided to write are custom formulas for milvus.
On the other hand, I will only consider the dense single vector situation, since other cases are complicated, and leaving them for further consideration.
Feel free to let me know if you have some problem with this design. :)

@cheriL
Copy link
Author

cheriL commented Nov 7, 2024

I agree that the multi-vector situation is quite complex. Thanks for your work on this project!

@zc277584121
Copy link
Collaborator

done: #18
@cheriL FYI

@zc277584121
Copy link
Collaborator

so i close this one

@zc277584121 zc277584121 closed this Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants