refactor: count items from hnsw index by shobhit9957 · Pull Request #1765 · docarray/docarray

shobhit9957 · 2023-08-25T00:12:04Z

Signed-off-by: Shobhit Kumar <[email protected]>

JoanFM · 2023-08-25T05:31:09Z

docarray/index/backends/hnswlib.py

+        Get the number of documents using the HNSW method.
+        """
+        # Access the first HNSW index from self._hnsw_indices
+        first_hnsw_index = self._hnsw_indices[0]


this is a dictionary, not a list, so u should not access it like this

JoanFM · 2023-08-25T05:31:52Z

Also, please remember that you need to signofff every commit so that we csn merge the PR

codecov · 2023-08-25T05:35:25Z

Codecov Report

Patch coverage: 100.00% and project coverage change: -1.05% ⚠️

Comparison is base (587ab5b) 83.74% compared to head (13a9ebf) 82.70%.
Report is 19 commits behind head on main.

❗ Current head 13a9ebf differs from pull request most recent head 436937b. Consider uploading reports for the commit 436937b to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1765      +/-   ##
==========================================
- Coverage   83.74%   82.70%   -1.05%     
==========================================
  Files         134      134              
  Lines        8845     8850       +5     
==========================================
- Hits         7407     7319      -88     
- Misses       1438     1531      +93

Flag	Coverage Δ
docarray	`82.70% <100.00%> (-1.05%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
docarray/index/backends/hnswlib.py	`60.70% <100.00%> (-13.58%)`	⬇️

... and 16 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

shobhit9957 · 2023-08-25T13:17:14Z

Hey, i'm so sorry joan. for the mistakes... I'll remember to sign off my every commit from now. and i'll try to solve this issue and commit again(signed off). Thanks Joan!

Signed-off-by: Shobhit Kumar <[email protected]>

JoanFM · 2023-08-25T15:32:45Z

Hey, i'm so sorry joan. for the mistakes... I'll remember to sign off my every commit from now. and i'll try to solve this issue and commit again(signed off). Thanks Joan!

Nothing to be sorrry about. This is about Open Source and supporting each other

JoanFM · 2023-08-25T15:35:49Z

docarray/index/backends/hnswlib.py

+        first_hnsw_index = self._hnsw_indices[first_index_key]
+        hnsw_num_docs = (
+            first_hnsw_index.get_document_count()
+        )  # Replace with actual method


this comment is not anymore needed

Signed-off-by: Shobhit Kumar <[email protected]>

shobhit9957 · 2023-08-28T03:09:38Z

Hey Joan, could you please check the commit, and if there are no issues in it, can you please merge it Joan. If there are any issues , please let me know Joan. I'll try to solve it.

JoanFM · 2023-08-28T08:05:33Z

Hey Joan, could you please check the commit, and if there are no issues in it, can you please merge it Joan. If there are any issues , please let me know Joan. I'll try to solve it.

The tests are failing as seen here https://github.com/docarray/docarray/actions/runs/5978323747/job/16260526492?pr=1765

I believe the method you use does not exist, please check what is the appropiate method name

docarray/index/backends/hnswlib.py

Signed-off-by: Shobhit Kumar <[email protected]>

JoanFM · 2023-08-28T12:01:59Z

docarray/index/backends/hnswlib.py

        first_index_key = next(iter(self._hnsw_indices))  # Get the first key
        first_hnsw_index = self._hnsw_indices[first_index_key]
-        hnsw_num_docs = first_hnsw_index.get_document_count()
+        hnsw_num_docs = first_hnsw_index.element_count()


it is not a method, it is an attribute

hey joan I've fixed this....can you please say me what is the issue again...so that the tests are failing...

You can check what is faling here https://github.com/docarray/docarray/actions/runs/6003026515/job/16306992708?pr=1765

And try to run the tests urself

Okey, I see that the test asserts the number of docs after deletion, and this HNSWlib element_count may not count deleted

Signed-off-by: Shobhit Kumar <[email protected]>

JoanFM · 2023-08-30T14:39:43Z

@shobhit9957 ,

I have realised about some challenges of this implementation, would u be able to join our discord to have better way to discuss how to tackle them?

https://discord.gg/gWEbQNcg

shobhit9957 · 2023-08-30T14:42:14Z

hey thanks joan, for the invitation of the discord, for sure I'll join joan. Thanks joan! let's fix this issue!

Signed-off-by: Shobhit Kumar <[email protected]>

JoanFM · 2023-09-03T11:59:55Z

docarray/index/backends/hnswlib.py


+    def _count_docs_in_hnswlib(self) -> int:
+        # Use HNSWlib's method to count documents
+        return self._hnsw_indices.element_count


remember from last iteration, this is a dictionary, not an index itself, u need to get the first index in the dictionary

Signed-off-by: Shobhit Kumar <[email protected]>

docarray/index/backends/hnswlib.py

Signed-off-by: Shobhit Kumar <[email protected]>

JoanFM · 2023-09-08T13:17:51Z

docarray/index/backends/hnswlib.py

+            self._num_docs = self._get_num_docs_hnsw()
        return self._num_docs

+    def is_index_empty(self) -> bool:


this method needs to be defined at parent class

Please remove this method here as it is not needed now

This reverts commit f12bf20.

Signed-off-by: Shobhit Kumar <[email protected]>

JoanFM · 2023-09-19T15:46:10Z

docarray/index/backends/hnswlib.py

+            self._num_docs = self._get_num_docs_hnsw()
        return self._num_docs

+    def is_index_empty(self) -> bool:


Please remove this method here as it is not needed now

JoanFM · 2023-09-19T15:46:35Z

docarray/index/backends/hnswlib.py

        """
        if self._num_docs == 0:
-            self._num_docs = self._get_num_docs_sqlite()
+            self._num_docs = self._get_num_docs_hnsw()


Remove this change

JoanFM · 2023-09-19T15:46:50Z

docarray/index/backends/hnswlib.py

@@ -428,9 +426,17 @@ def num_docs(self) -> int:
        Get the number of documents.
        """
        if self._num_docs == 0:


Change this call to use the new method

Signed-off-by: Shobhit Kumar <[email protected]>

JoanFM · 2023-09-20T13:14:15Z

docarray/index/backends/hnswlib.py

-        :return: True if the index is empty, False otherwise.
-        """
-        return self.num_docs() == 0
+        if self.is_index_empty == 0:


this does not make sense, is_index_empty should not be compared to 0, it already gives a boolean

JoanFM · 2023-09-20T20:04:08Z

Let's take over again when I merge the first step of this epic #1801

shobhit9957 · 2023-09-22T00:42:53Z

Okay!, so now should I commit on #1801 ?

JoanFM · 2023-09-22T07:21:22Z

Okay!, so now should I commit on #1801 ?

No, you should create a new PR with the next steps.

shobhit9957 · 2023-09-22T15:59:04Z

Okay!, got it.

shobhit9957 added 3 commits August 24, 2023 20:12

solved the good first issue in the hnswlib.py file

bf6382e

Edited the code.

9466389

fix: docarray#1764, assigned the first indices

83fa2b7

Signed-off-by: Shobhit Kumar <[email protected]>

JoanFM requested changes Aug 25, 2023

View reviewed changes

fix : docarray#1764 Fix accessing HNSW index document count

45f4ae4

Signed-off-by: Shobhit Kumar <[email protected]>

JoanFM requested changes Aug 25, 2023

View reviewed changes

fix: docarray#1764 removed the comment

042d5c3

Signed-off-by: Shobhit Kumar <[email protected]>

JoanFM changed the title ~~Fix: #1764 , assigned the first indices.~~ refactor: count items from hnsw index Aug 28, 2023

JoanFM requested changes Aug 28, 2023

View reviewed changes

docarray/index/backends/hnswlib.py Outdated Show resolved Hide resolved

fixes: docarray#1765, implementing element_count()

8ecffda

Signed-off-by: Shobhit Kumar <[email protected]>

JoanFM requested changes Aug 28, 2023

View reviewed changes

fixes: docarray#1765 added the attribute(in the hnswlib.py)

13a9ebf

Signed-off-by: Shobhit Kumar <[email protected]>

fixes docarray#1765

0b428c6

Signed-off-by: Shobhit Kumar <[email protected]>

JoanFM requested changes Sep 3, 2023

View reviewed changes

shobhit9957 added 2 commits September 4, 2023 05:40

fixes: 1765 got the first index in the dictionary

803fc5d

Signed-off-by: Shobhit Kumar <[email protected]>

fixes: docarray#1765, step 1

1057805

Signed-off-by: Shobhit Kumar <[email protected]>

JoanFM requested changes Sep 5, 2023

View reviewed changes

docarray/index/backends/hnswlib.py Outdated Show resolved Hide resolved

fixes: docarray#1765 step 1 again

f12bf20

Signed-off-by: Shobhit Kumar <[email protected]>

JoanFM requested changes Sep 8, 2023

View reviewed changes

shobhit9957 added 3 commits September 19, 2023 17:19

fixes: docarray#1765

81988c1

Revert "fixes: docarray#1765 step 1 again"

d576e60

This reverts commit f12bf20.

fixes: docarray#1765

229d73f

Signed-off-by: Shobhit Kumar <[email protected]>

JoanFM requested changes Sep 19, 2023

View reviewed changes

shobhit9957 added 2 commits September 20, 2023 18:30

fixes: docarray#1765

678f1dd

fixes: docarray#1765

436937b

Signed-off-by: Shobhit Kumar <[email protected]>

JoanFM requested changes Sep 20, 2023

View reviewed changes

JoanFM closed this Sep 20, 2023

Conversation

shobhit9957 commented Aug 25, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoanFM commented Aug 25, 2023

Uh oh!

codecov bot commented Aug 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

shobhit9957 commented Aug 25, 2023

Uh oh!

JoanFM commented Aug 25, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shobhit9957 commented Aug 28, 2023

Uh oh!

JoanFM commented Aug 28, 2023

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoanFM commented Aug 30, 2023

Uh oh!

shobhit9957 commented Aug 30, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JoanFM commented Sep 20, 2023

Uh oh!

shobhit9957 commented Sep 22, 2023

Uh oh!

JoanFM commented Sep 22, 2023

Uh oh!

shobhit9957 commented Sep 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Aug 25, 2023 •

edited

Loading