fix: error from hybrid search's bm25 search by field

[hsm207]

fixes #1612

[hsm207]

@JoanFM @AnneYang720 Commit 3a6a293 is causing the the following tests in the test_subindex.py file to fail:

1. test_subindex_index
2. test_find_subindex
3. test_subindex_del

is the implementation of the subindex in weaviate somehow dependent on the ID field being a 'string' datatype?

[JoanFM]

@JoanFM @AnneYang720 Commit 3a6a293 is causing the the following tests in the test_subindex.py file to fail:

1. test_subindex_index
2. test_find_subindex
3. test_subindex_del

is the implementation of the subindex in weaviate somehow dependent on the ID field being a 'string' datatype?

Maybe implicitly there is. How does it fail?

[JoanFM]

@hsm207 check this comment:

TODO: add more types and figure out how to handle text vs string type on the top of the file, it may be related to this?

[hsm207]

How does it fail?

it's always related to the length of the documents returned. E.g. in:

def test_subindex_get(index):
    doc = index['1']
    assert type(doc) == MyDoc
    assert doc.id == '1'

    assert len(doc.docs) == 5

the value of docs.docs is:

0: SimpleDoc(id='docs-1-4', simple_tens=NdArray([5, 5, 5, 5, 5, 5, 5, 5, 5, 5]), simple_text='hello 4')
1: SimpleDoc(id='docs-1-0', simple_tens=NdArray([1, 1, 1, 1, 1, 1, 1, 1, 1, 1]), simple_text='hello 0')
2: SimpleDoc(id='docs-1-1', simple_tens=NdArray([2, 2, 2, 2, 2, 2, 2, 2, 2, 2]), simple_text='hello 1')
3: SimpleDoc(id='docs-3-1', simple_tens=NdArray([2, 2, 2, 2, 2, 2, 2, 2, 2, 2]), simple_text='hello 1')
4: SimpleDoc(id='docs-1-3', simple_tens=NdArray([4, 4, 4, 4, 4, 4, 4, 4, 4, 4]), simple_text='hello 3')
5: SimpleDoc(id='docs-0-1', simple_tens=NdArray([2, 2, 2, 2, 2, 2, 2, 2, 2, 2]), simple_text='hello 1')
6: SimpleDoc(id='docs-1-2', simple_tens=NdArray([3, 3, 3, 3, 3, 3, 3, 3, 3, 3]), simple_text='hello 2')
7: SimpleDoc(id='docs-2-1', simple_tens=NdArray([2, 2, 2, 2, 2, 2, 2, 2, 2, 2]), simple_text='hello 1')
8: SimpleDoc(id='docs-4-1', simple_tens=NdArray([2, 2, 2, 2, 2, 2, 2, 2, 2, 2]), simple_text='hello 1')

before it was:

0: SimpleDoc(id='docs-1-1', simple_tens=NdArray([2, 2, 2, 2, 2, 2, 2, 2, 2, 2]), simple_text='hello 1')
1: SimpleDoc(id='docs-1-0', simple_tens=NdArray([1, 1, 1, 1, 1, 1, 1, 1, 1, 1]), simple_text='hello 0')
2: SimpleDoc(id='docs-1-4', simple_tens=NdArray([5, 5, 5, 5, 5, 5, 5, 5, 5, 5]), simple_text='hello 4')
3: SimpleDoc(id='docs-1-3', simple_tens=NdArray([4, 4, 4, 4, 4, 4, 4, 4, 4, 4]), simple_text='hello 3')
4: SimpleDoc(id='docs-1-2', simple_tens=NdArray([3, 3, 3, 3, 3, 3, 3, 3, 3, 3]), simple_text='hello 2')

Does _get_items work differently with nested documents? I don't think this is the case based on this part of the codebase, unless it's overriden elsewhere

check this comment
TODO: add more types and figure out how to handle text vs string type on the top of the file, it may be related to this?
it could be due to how the id is tokenized (field (before) vs word (now)) .

Can you confirm that nested documents don't somehow override _get_items?

[JoanFM]

How does it fail?

it's always related to the length of the documents returned. E.g. in:

def test_subindex_get(index):
    doc = index['1']
    assert type(doc) == MyDoc
    assert doc.id == '1'

    assert len(doc.docs) == 5

the value of docs.docs is:

0: SimpleDoc(id='docs-1-4', simple_tens=NdArray([5, 5, 5, 5, 5, 5, 5, 5, 5, 5]), simple_text='hello 4')
1: SimpleDoc(id='docs-1-0', simple_tens=NdArray([1, 1, 1, 1, 1, 1, 1, 1, 1, 1]), simple_text='hello 0')
2: SimpleDoc(id='docs-1-1', simple_tens=NdArray([2, 2, 2, 2, 2, 2, 2, 2, 2, 2]), simple_text='hello 1')
3: SimpleDoc(id='docs-3-1', simple_tens=NdArray([2, 2, 2, 2, 2, 2, 2, 2, 2, 2]), simple_text='hello 1')
4: SimpleDoc(id='docs-1-3', simple_tens=NdArray([4, 4, 4, 4, 4, 4, 4, 4, 4, 4]), simple_text='hello 3')
5: SimpleDoc(id='docs-0-1', simple_tens=NdArray([2, 2, 2, 2, 2, 2, 2, 2, 2, 2]), simple_text='hello 1')
6: SimpleDoc(id='docs-1-2', simple_tens=NdArray([3, 3, 3, 3, 3, 3, 3, 3, 3, 3]), simple_text='hello 2')
7: SimpleDoc(id='docs-2-1', simple_tens=NdArray([2, 2, 2, 2, 2, 2, 2, 2, 2, 2]), simple_text='hello 1')
8: SimpleDoc(id='docs-4-1', simple_tens=NdArray([2, 2, 2, 2, 2, 2, 2, 2, 2, 2]), simple_text='hello 1')

before it was:

0: SimpleDoc(id='docs-1-1', simple_tens=NdArray([2, 2, 2, 2, 2, 2, 2, 2, 2, 2]), simple_text='hello 1')
1: SimpleDoc(id='docs-1-0', simple_tens=NdArray([1, 1, 1, 1, 1, 1, 1, 1, 1, 1]), simple_text='hello 0')
2: SimpleDoc(id='docs-1-4', simple_tens=NdArray([5, 5, 5, 5, 5, 5, 5, 5, 5, 5]), simple_text='hello 4')
3: SimpleDoc(id='docs-1-3', simple_tens=NdArray([4, 4, 4, 4, 4, 4, 4, 4, 4, 4]), simple_text='hello 3')
4: SimpleDoc(id='docs-1-2', simple_tens=NdArray([3, 3, 3, 3, 3, 3, 3, 3, 3, 3]), simple_text='hello 2')

Does _get_items work differently with nested documents? I don't think this is the case based on this part of the codebase, unless it's overriden elsewhere

check this comment
TODO: add more types and figure out how to handle text vs string type on the top of the file, it may be related to this?
it could be due to how the id is tokenized (field (before) vs word (now)) .

Can you confirm that nested documents don't somehow override _get_items?

Maybe @JohannesMessner knows better

[JoanFM]

Check this:

    def _filter_by_parent_id(self, id: str) -> Optional[List[str]]:
        results = (
            self._client.query.get(self._db_config.index_name, ['docarrayid'])
            .with_where(
                {'path': ['parent_id'], 'operator': 'Equal', 'valueString': f'{id}'}
            )
            .do()
        )

        ids = [
            res['docarrayid']
            for res in results['data']['Get'][self._db_config.index_name]
        ]
        return ids

I guess the fact that the ID column is not a string but text may affect

[hsm207]

@JoanFM I think I found the reason for the failure. See #1698. I'll investigate further when I'm back from vacation next week

[JoanFM]

@JoanFM I think I found the reason for the failure. See #1698. I'll investigate further when I'm back from vacation next week

So the issue was that the ID is being tokenized then?

[hsm207]

@JoanFM I think I found the reason for the failure. See #1698. I'll investigate further when I'm back from vacation next week

So the issue was that the ID is being tokenized then?

assuming that nested documents isn't somehow override _get_items, then yes, I think this is the cause.

[JohannesMessner]

@JoanFM I think I found the reason for the failure. See #1698. I'll investigate further when I'm back from vacation next week

So the issue was that the ID is being tokenized then?

assuming that nested documents isn't somehow override _get_items, then yes, I think this is the cause.

If there are nested Documents it performs the same __getitem__/__get_items call on the nested docs as it does on the root docs. So there should be no overriding, unless a particular backend does something wonky.

[codecov]

Codecov Report

Patch coverage: 50.00% and project coverage change: -2.37% ⚠️

Comparison is base (587ab5b) 83.74% compared to head (1a381f1) 81.37%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1692      +/-   ##
==========================================
- Coverage   83.74%   81.37%   -2.37%     
==========================================
  Files         134      134              
  Lines        8845     8848       +3     
==========================================
- Hits         7407     7200     -207     
- Misses       1438     1648     +210     

Flag	Coverage Δ
docarray	81.37% <50.00%> (-2.37%)	⬇️

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

Files Changed	Coverage Δ
docarray/index/backends/weaviate.py	52.57% <50.00%> (-42.47%)	⬇️

... and 16 files with indirect coverage changes

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

[hsm207]

@JohannesMessner @JoanFM

I made the commit to set the tokenization for "id" to field and now the tests is failing at this line:

assert len(doc.list_docs[0].docs) == 5

the value of doc.list_docs[0].docs is:

What is the correct value and how is it built differently compared to doc.docs and doc.list_docs?

[JoanFM]

@JohannesMessner @JoanFM

I made the commit to set the tokenization for "id" to field and now the tests is failing at this line:

assert len(doc.list_docs[0].docs) == 5

the value of doc.list_docs[0].docs is:

What is the correct value and how is it built differently compared to `doc.docs` and `doc.list_docs`?

I do not know, @JohannesMessner may be better to judge.

However, I do not like at all that test folder, since each test cannot be run individually, as they all depend on the same fixture. I did a refactoring of that test for HNSWLib, I will see if I can do it for this one

[JoanFM]

Hey @hsm207,

what is the ststus of this issue?

[hsm207]

hey @JoanFM

I haven't been able to delve into the nested document feature yet due to current commitments on other projects. I've moved this to my backlog since I don't see an opportunity to work on this until at least mid next month.