Iterators and range_filter error (when no range filter is specified)

[PiercarloSlavazza]

I started to use the search iterators.

I am using:

• milvus sdk java: 2.4.0
• milvus: 2.3.3 and v2.4.0-rc.1

I create the SearchIteratorParam in the following way:

SearchIteratorParam.newBuilder()
                .withCollectionName(metadata.getCollectionName())
                .withMetricType(MetricType.IP)
                .withOutFields(Arrays.asList(
                        METADATA_FIELD_ID_COURT.getKeyId(),
                        METADATA_FIELD_ID_PDD_SETTING_JUDGEMENTS_YEARS.getKeyId(),
                        METADATA_FIELD_ID_SENTENCE_UUID.getKeyId(),
                        METADATA_FIELD_ID_TAGS.getKeyId(),
                        METADATA_FIELD_ID_TEXT_TYPE.getKeyId(),
                        METADATA_FIELD_ID_TEXT_UUID_PK.getKeyId(),
                        METADATA_FIELD_ID_YEAR.getKeyId()
                ))
                .withFloatVectors(singletonList(embeddings.getVector().stream().map(Double::floatValue).toList()))
                .withBatchSize(500L)
                .withVectorFieldName(METADATA_FIELD_ID_EMBEDDINGS.getKeyId())

and afterwards I experience the following weird behaviour:

• on machine n. 1 (macOS), with Milvus 2.3.3 ⇒ everything works fine
• on machine n. 2 (Ubuntu) with Milvus v2.4.0-rc.1, I get the following error: io.milvus.v2.exception.MilvusClientException: range_filter must be less than radius for L2/HAMMING/JACCARD, range_filter:0.853851, radius:0.809344: invalid parameter

This is very strange because:

1. I used IP
2. I do not set any radius/rangefilter parameter - so I suppose that they come from some weird defaults
3. even if I switch on machine n. 2 to Milvus 2.3.3, I still get the error (maybe defaults are written somewhere in the index/database?)

Moreover, if I try to set radius/rangefilter so that the defaults do not kick in, such as in

                .withParams("""
                        {
                            "radius": 1.0,
                            "range_filter" : 1.0
                        }""")

• on machine n. 1 (milvus 2.3.3) I get the error: lass java.lang.Double cannot be cast to class java.lang.Float (java.lang.Double and java.lang.Float are in module java.base of loader 'bootstrap')
• on machine n. 2 (milvus v2.4.0-rc.1), nothing changes, I still get "range_filter must be less than radius"

I have only found this potentially related issue: milvus-io/milvus#29305

Any hint?

Thank you.

[PiercarloSlavazza]

Tried Milvus 2.4.0 as well - still getting the error.

[PiercarloSlavazza]

Just in case someone will experience this very same issue, I managed to solve it by:

• creating a brand new instance of Milvus v 2.3.3
• connecting via milvus sdk v2.4.0

This wasy the iterators are working fine.

I assume that sdk 2.4.0 iterators have been tested with Milvus 2.4.0 - so I think that it is something on my side, but I wasn't able to track it down (apart going back to Milvus 2.3.3)…

[yhmo]

A valid range search param for L2 metric should be:

String params = "{\"radius\": 1.0, \"range_filter\": 0.5}";
SearchIteratorParam.Builder searchIteratorParamBuilder = SearchIteratorParam.newBuilder()
                ......
                .withParams(params)
                .withMetricType(MetricType.L2);

• With L2 metric type, the value of range_filter should be less than radius.
• With IP metric type, the value of radius should be less than range_filter.

[PiercarloSlavazza]

Thanks for your clarification, but my search does not use L2 - it actually uses .withMetricType(MetricType.IP) (see 1st comment).

[yhmo]

But you set "radius"=1.0 and "range_filter"=1.0, the two parameters cannot be equal, neither with IP nor L2.

With IP metric type, the value of radius should be less than range_filter.

String params = "{\"radius\": 0.5, \"range_filter\": 0.8}";
SearchIteratorParam.Builder searchIteratorParamBuilder = SearchIteratorParam.newBuilder()
                ......
                .withParams(params)
                .withMetricType(MetricType.IP);

[PiercarloSlavazza]

I see - thank you for pointing it.

However:

• I resorted to setting the params only later on, in order to try to circumvent the issue that occurred in first place: I will try again following your suggestion
• so, please consider that the problem occurred in the first place when I tried to execute the search without having specified any parameter: it alwasy worked, I works with Milvus 2.3.3, it doesn't with Milvus 2.4.* - maybe it is by design, are able to confirm it?
• lastly, even if it is everything by design, the error message is at least misleading: I used IP, and the error message I got was referring to L2

[xiaofan-luan]

I thought it is because you don't specify metrics when you create index. and by default it is L2.
ignore the metrics when search, it only helps on verify the build index metrics

[PiercarloSlavazza]

Guys, I finally found the bugs.

There are - according to my investigations - two (unrelated) bugs:

Bug n.1

In statement that starts at this line:

milvus-sdk-java/src/main/java/io/milvus/orm/iterator/SearchIterator.java

Line 221 in bf981d0

SearchParam.Builder searchParamBuilder = SearchParam.newBuilder()

the metric type is NOT initialized using the searchIteratorParam.getMetricType().

This affects ONLY the iterator.next calls subsequent to the first one: the SDK computes correctly the new params, BUT, given that the metric type is not re-initialized AND I am using IP which is NOT the default, the server thinks that the intended metric is L2 (the default), and therefore it complains that the range and radius paramters are inverted - which indeed are NOT, but only if the correct metric type is reported to the server.

Bug n. 2

This bug is actually unrelated to bug n.1 - it just happened to occur while I was trying hard to find a workaround to bug n. 1 (well, before I understood the very nature of that first issue, of course).

In this line:

milvus-sdk-java/src/main/java/io/milvus/orm/iterator/SearchIterator.java

Line 143 in bf981d0

float radius = (float) params.get(RADIUS);

the result of params.get(RADIUS) is cast to a Float - but this leads to a casting error because params.get(RADIUS) is read as a Double, which of course cannot be just cast to a Float. BTW the parameter is read as a Double because this is parsed via JacksonUtils.fromJson(searchIteratorParam.getParams(), new TypeReference<Map<String, Object>>(){}), and Jackson wil always read a real number as a Double in this case, and this behaviour is not - to my knowledge - configurable.

So, if you confirm my findings, I will report the Bug. n. 2 in another issue.

I would like to highlight that bug n. 1 is blocking for whoever wants to use iterators with IP metric in Java.

[xiaofan-luan]

Guys, I finally found the bugs.

There are - according to my investigations - two (unrelated) bugs:

Bug n.1

In statement that starts at this line:

milvus-sdk-java/src/main/java/io/milvus/orm/iterator/SearchIterator.java

Line 221 in bf981d0

SearchParam.Builder searchParamBuilder = SearchParam.newBuilder()

the metric type is NOT initialized using the searchIteratorParam.getMetricType().
This affects ONLY the iterator.next calls subsequent to the first one: the SDK computes correctly the new params, BUT, given that the metric type is not re-initialized AND I am using IP which is NOT the default, the server thinks that the intended metric is L2 (the default), and therefore it complains that the range and radius paramters are inverted - which indeed are NOT, but only if the correct metric type is reported to the server.

Bug n. 2

This bug is actually unrelated to bug n.1 - it just happened to occur while I was trying hard to find a workaround to bug n. 1 (well, before I understood the very nature of that first issue, of course).

In this line:

milvus-sdk-java/src/main/java/io/milvus/orm/iterator/SearchIterator.java

Line 143 in bf981d0

float radius = (float) params.get(RADIUS);

the result of params.get(RADIUS) is cast to a Float - but this leads to a casting error because params.get(RADIUS) is read as a Double, which of course cannot be just cast to a Float. BTW the parameter is read as a Double because this is parsed via JacksonUtils.fromJson(searchIteratorParam.getParams(), new TypeReference<Map<String, Object>>(){}), and Jackson wil always read a real number as a Double in this case, and this behaviour is not - to my knowledge - configurable.
So, if you confirm my findings, I will report the Bug. n. 2 in another issue.

I would like to highlight that bug n. 1 is blocking for whoever wants to use iterators with IP metric in Java.

Thanks for reporting the bugs.
We will fix bug 1 as soon as possible and maybe you want to take bug 2?
Questions:
what's the reason of cast could causing a problem?

[PiercarloSlavazza]

Hi @xiaofan-luan ,

We will fix bug 1 as soon as possible

Actually I already attached a PR fixing bug n. 1 - the fix was trivial, but I am a bit in the rush and wasn't even able to run the unit tests, let alone adding a new one… so I would like to ask if someone could please add the related unit test, thank you.

and maybe you want to take bug 2?

I will do the fix - ditto for the unit tests.

what's the reason of cast could causing a problem?

I am not sure I am getting your point… reason of what, exactly? The cast from Double to Float is forbidden by the VM, if done as it has been done in the code base (the root of all evil is of course in first place having used a Map of Objects… imho choices like this nullify the whole point of having a type system, and in the end lead to "undetectable" bugs like this one) - the way to go is Double.floatValue(), so in this case ((Double) params.get(RADIUS)).floatValue().

[xiaofan-luan]

pr looks good to me, please fix the DCO so I can merge the PR, thanks for the contribution.

[yhmo]

I just tried the IteratorExample.java, I got the exception about the cast:

Exception in thread "main" java.lang.RuntimeException: class java.lang.Double cannot be cast to class java.lang.Float (java.lang.Double and java.lang.Float are in module java.base of loader 'bootstrap')
	at io.milvus.CommonUtils.handleResponseStatus(CommonUtils.java:34)
	at io.milvus.IteratorExample.getSearchIterator(IteratorExample.java:308)

I agree the executeNextSearch() should pass the metricType to SearchParam, it is a bug.
Wait @lentitude2tk to condirm.

[xiaofan-luan]

Hi @xiaofan-luan ,

We will fix bug 1 as soon as possible

Actually I already attached a PR fixing bug n. 1 - the fix was trivial, but I am a bit in the rush and wasn't even able to run the unit tests, let alone adding a new one… so I would like to ask if someone could please add the related unit test, thank you.

and maybe you want to take bug 2?

I will do the fix - ditto for the unit tests.

what's the reason of cast could causing a problem?

I am not sure I am getting your point… reason of what, exactly? The cast from Double to Float is forbidden by the VM, if done as it has been done in the code base (the root of all evil is of course in first place having used a Map of Objects… imho choices like this nullify the whole point of having a type system, and in the end lead to "undetectable" bugs like this one) - the way to go is Double.floatValue(), so in this case ((Double) params.get(RADIUS)).floatValue().

Thanks for the clarification . I think the reason is not cast double to int, but to convert a object to primitive directly. make sense to me

[PiercarloSlavazza]

I think the reason is not cast double to int, but to convert a object to primitive directly. make sense to me

You are wrong - look at this code from JShell - no primitives here, and the error message is very clear:

jshell> (Float) new Double(0.1)
|  Warning:
|  Double(double) in java.lang.Double has been deprecated and marked for removal
|  (Float) new Double(0.1)
|          ^-------------^
|  Error:
|  incompatible types: java.lang.Double cannot be converted to java.lang.Float
|  (Float) new Double(0.1)
|          ^-------------^

[yhmo]

@PiercarloSlavazza
Just confirm, the metricType for search() has been deprecated from v2.3 on the server side. Metric type is determined by the CreateIndexParam when you call createIndex(). So, we can pass None to SearchParam, just let the server get the metric type from index.

So, if you want to use MetricType.IP, you can specify it to the index:

R<RpcStatus> response = milvusClient.createIndex(CreateIndexParam.newBuilder()
                .withCollectionName(COLLECTION_NAME)
                .withFieldName(VECTOR_FIELD)
                .withIndexName(INDEX_NAME)
                .withIndexType(INDEX_TYPE)
                .withMetricType(MetricType.IP)
                .withExtraParam(INDEX_PARAM)
                .withSyncMode(Boolean.TRUE)
                .build());

And we don't need to change the code of SearchIterator.

[PiercarloSlavazza]

Hi,

• added signoff to the PR
• added patch for the cast bug (tracked in in its own issue When a SearchIteratorParam is initialized via withParams, calling build() raises an error #887 )
• @yhmo I already have the call to .withMetricType(MetricType.IP) in my index initialization - so the patches are actually needed

[yhmo]

@PiercarloSlavazza
The pr has been merged.
Could you cherry-pick it to the 2.3 branch and master branch?

[PiercarloSlavazza]

@yhmo

• done for master
• 2.3 has issues: cherry picking my patch (and the patch that fixes the casting issue) cause a conflict - but I am not able to resolve it with confidence because it's about code that I do not know what is doing…

[yhmo]

The code logic is a bit different between 2.3 and 2.4 and cause the conflict. I have merged the pr to 2.3 branch.