Skip to content

fix: find_and_filter for inmemory#1642

Merged
jupyterjazz merged 2 commits intomainfrom
fix-filter-and-find
Jun 13, 2023
Merged

fix: find_and_filter for inmemory#1642
jupyterjazz merged 2 commits intomainfrom
fix-filter-and-find

Conversation

@jupyterjazz
Copy link
Contributor

@jupyterjazz jupyterjazz commented Jun 12, 2023 •
edited
Loading

#1640

Hybrid search (find+filter) for InMemoryExactNNIndex was prioritizing low similarities (lower scores) for returned matches. Fixed by adding an option to sort matches in a reverse order based on their scores.

# prepare a query
q_doc = MyDoc(embedding=np.random.rand(128), text='query')

query = (
    db.build_query()
    .find(query=q_doc, search_field='embedding')
    .filter(filter_query={'text': {'$exists': True}})
    .build()
)

results = db.execute_query(query)
# Before: results was sorted from worst to best matches
# Now: It's sorted in the correct order, showing better matches first

samsja
samsja reviewed Jun 13, 2023
View reviewed changes
docarray/index/backends/helper.py Outdated
Comment on lines +31 to +40

Args:
doc_index: Document index instance.
Either InMemoryExactNNIndex or HnswDocumentIndex.
query: Dictionary containing search and filtering configuration.
reverse_order: Flag indicating whether to sort in descending order. If set to
False (default), the sorting will be in ascending order.

Returns:
Sorted documents and their corresponding scores.
Copy link
Member

@samsja samsja Jun 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not the right style for docstring

Copy link
Contributor Author

@jupyterjazz jupyterjazz Jun 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got too used to langchain docstrings lol

samsja
samsja requested changes Jun 13, 2023
View reviewed changes
Copy link
Member

@samsja samsja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good.

Tho I have to say it is unclear why adding this reverse sorting fix the original problem. Can you add a comment somewhere in the code to explain it ?

FYI: docstring are not in the right format

@jupyterjazz
docs: update docstring
2ec2e80 
Signed-off-by: jupyterjazz <[email protected]>
@jupyterjazz jupyterjazz requested a review from samsja June 13, 2023 07:42
@github-actions
Copy link

github-actions bot commented Jun 13, 2023

📝 Docs are deployed on https://ft-fix-filter-and-find--jina-docs.netlify.app 🎉

@jupyterjazz jupyterjazz merged commit f36c621 into main Jun 13, 2023
@jupyterjazz jupyterjazz deleted the fix-filter-and-find branch June 13, 2023 08:06
@samsja samsja mentioned this pull request Jun 19, 2023
Closed
@scott-martens scott-martens mentioned this pull request Jun 20, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@samsja samsja samsja approved these changes

@JoanFM JoanFM Awaiting requested review from JoanFM

@JohannesMessner JohannesMessner Awaiting requested review from JohannesMessner

@maxwelljin maxwelljin Awaiting requested review from maxwelljin

Assignees

No one assigned

Labels

area/core area/testing size/s

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jupyterjazz @samsja