Skip to content

feat: find function #931

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.

Sign up for GitHub

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

Jump to bottom
Merged
JohannesMessner merged 20 commits into from
Dec 14, 2022
Merged

feat: find function #931

JohannesMessner merged 20 commits into from
Dec 14, 2022

Conversation

@JohannesMessner
Copy link
Member

@JohannesMessner JohannesMessner commented Dec 12, 2022 •
edited
Loading

Goals:

Create find() function:

from docarray import DocumentArray, Document
from docarray.utility import find, find_batched
from docarray.typing import TorchTensor

class MyDoc(Document):
    tensor: TorchTensor

da = DocumentArray[MyDoc](MyDoc(tensor=torch.rand(128)) for _ in range(10))

matches, scores = find(da, MyDoc(tensor=torch.rand(128)), embedding_field='tensor', metric='cosine_sim')

batched_query = DocumentArray[MyDoc](
    [MyDoc(tensor=torch.rand(128)) for _ in range(3)]
)
results = find_batched(da, batched_query, embedding_field='tensor', metric='cosine_sim')
assert len(results) == 3
for matches_i, socres_i in results:
    ...

TODO

  • implement find
    • for torch
    • for numpy
    • allow docarray as query
  • refactor: Create classes for backend operations. Edit: Separate PR
  • refactor: let types define the backend they belong to. Edit: Separate PR
  • consider having find and find_batched explicitly
  • user defined callable as distance function. Edit: not doing in first iteration
  • tests
  • Documentation (docstrings)
  • Optional: batching Edit: not in this PR
  • Optional: nested find.Edit: not doing in first iteration
  • Optional: other features from current docarray. Edit: not doing in first iteration

@JohannesMessner
refactor: make abstract tensor parametrization modular
1addc11 
Signed-off-by: Johannes Messner <[email protected]>
@JohannesMessner
feat: add type for torch embedding
b0b8497 
Signed-off-by: Johannes Messner <[email protected]>
@JohannesMessner
feat: embedding type for ndarray
68f3914 
Signed-off-by: Johannes Messner <[email protected]>
@JohannesMessner
fix: fix general embedding type
953b690 
Signed-off-by: Johannes Messner <[email protected]>
@JohannesMessner
test: update tests
766f6b9 
Signed-off-by: Johannes Messner <[email protected]>
@JohannesMessner
Merge remote-tracking branch 'origin/feat-rewrite-v2' into feat-rewri…
9dface7 
…te-v2
@JohannesMessner
Merge remote-tracking branch 'origin/feat-rewrite-v2' into feat-rewri…
ed328e8 
…te-v2

# Conflicts:
#	docarray/typing/__init__.py
#	docarray/typing/tensor/__init__.py
#	docarray/typing/tensor/abstract_tensor.py
#	docarray/typing/tensor/embedding.py
#	tests/integrations/typing/test_torch_tensor.py
@JohannesMessner
feat: find function
cd7087b 
Signed-off-by: Johannes Messner <[email protected]>
@JohannesMessner JohannesMessner self-assigned this Dec 12, 2022
@JohannesMessner JohannesMessner added the DocArray v2 This issue is part of the rewrite; not to be merged into main label Dec 12, 2022
@JohannesMessner JohannesMessner linked an issue Dec 12, 2022 that may be closed by this pull request
create a find function that operate on DocumentArray #926
Closed
@JohannesMessner
test: add tests for find
1ed7d93 
Signed-off-by: Johannes Messner <[email protected]>
@JohannesMessner
test: add test for topk
0f7068f 
Signed-off-by: Johannes Messner <[email protected]>
@JohannesMessner
feat: add batched find
d5c9ef9 
Signed-off-by: Johannes Messner <[email protected]>
@JohannesMessner
test: add more tests
11cb7f2 
Signed-off-by: Johannes Messner <[email protected]>
@github-actions github-actions bot added size/xl and removed size/l labels Dec 13, 2022
@github-actions
Copy link

github-actions bot commented Dec 13, 2022

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@JohannesMessner
docs: improve docstrings
ba56987 
Signed-off-by: Johannes Messner <[email protected]>
@JohannesMessner
docs: improve docstrings
840ef79 
Signed-off-by: Johannes Messner <[email protected]>
@JohannesMessner JohannesMessner mentioned this pull request Dec 13, 2022
Closed
47 tasks
@JohannesMessner
fix: mypy and some comments
6e48cdb 
Signed-off-by: Johannes Messner <[email protected]>
@github-actions
Copy link

github-actions bot commented Dec 14, 2022

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@JohannesMessner JohannesMessner marked this pull request as ready for review December 14, 2022 09:24
@github-actions
Copy link

github-actions bot commented Dec 14, 2022

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

JohannesMessner
JohannesMessner commented Dec 14, 2022
View reviewed changes
docarray/utility/find.py
limit: int = 10,
device: Optional[str] = None,
descending: Optional[bool] = None,
) -> List[FindResult]:
Copy link
Member Author

@JohannesMessner JohannesMessner Dec 14, 2022

Choose a reason for hiding this comment

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

Is this a good return format?

@JohannesMessner
Copy link
Member Author

JohannesMessner commented Dec 14, 2022

Note that the file structure is not great atm, this will also change with the PR about computational backends

samsja
samsja reviewed Dec 14, 2022
View reviewed changes
@JohannesMessner
refactor: address code review
b5deadc 
Signed-off-by: Johannes Messner <[email protected]>
@github-actions
Copy link

github-actions bot commented Dec 14, 2022

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@JohannesMessner JohannesMessner mentioned this pull request Dec 14, 2022
Merged
@JohannesMessner
fix: suppress mypy
73e633e 
Signed-off-by: Johannes Messner <[email protected]>
@github-actions
Copy link

github-actions bot commented Dec 14, 2022

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@github-actions
Copy link

github-actions bot commented Dec 14, 2022

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

@JohannesMessner JohannesMessner merged commit c7dfb90 into feat-rewrite-v2 Dec 14, 2022
@JohannesMessner JohannesMessner deleted the feat-find branch December 14, 2022 13:26
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

Assignees

@JohannesMessner JohannesMessner

Labels

area/core area/testing area/typing DocArray v2 This issue is part of the rewrite; not to be merged into main size/xl

Projects

DocArray backlog
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

create a find function that operate on DocumentArray

3 participants

@JohannesMessner @samsja