ISPN-15390 Allow to project the query score #11679

fax4ever · 2024-01-15T15:18:36Z

https://issues.redhat.com/browse/ISPN-15390

~~Draft still to test/address: broadcast queries, remote queries.~~ done!

fax4ever · 2024-01-16T09:08:58Z

@yrodiere @marko-bekhta I don't if you want to have a look at it

marko-bekhta

I didn't find aa lot of Hibernate Search-related things in this PR so cannot add much, but looks nice overall 😃

marko-bekhta · 2024-01-16T09:44:17Z

query/src/test/java/org/infinispan/query/projection/ScoreProjectionTest.java

+      SearchSession mappingSession = searchMapping.getMappingSession();
+      SearchScope<Game> scope = mappingSession.scope(Game.class);
+
+      SearchProjection<List<?>> projection = scope.projection().composite(scope.projection().score(), scope.projection().entity()).toProjection();


Just wanted to share that not so long ago, Hibernate Search introduced a way to bind this type of projections like:

@ProjectionConstructor public record MyProjection(@ScoreProjection float score, @EntityProjection Game entity) { } // and then SearchProjection<MyProjection> projection = scope.projection().composite().as( MyProjection.class ).toProjection();

Not that it is needed here, but maybe you'll find it useful in some future tests 😃
See https://docs.jboss.org/hibernate/stable/search/reference/en-US/html_single/#search-dsl-projection-entity-mapping

Thank you @marko-bekhta nice API. Thanks for sharing it!

yrodiere · 2024-01-16T09:35:27Z

query/src/main/java/org/infinispan/query/dsl/embedded/impl/SearchQueryBuilder.java

@@ -72,15 +71,12 @@ public LuceneSearchQuery<List<Object>> keyAndEntity() {
      SearchProjectionFactory<EntityReference, ?> projectionFactory = scope.projection();
      SearchProjection<?>[] searchProjections = new SearchProjection<?>[]{
            projectionFactory.entityReference().toProjection(),
-            projectionFactory.entity().toProjection()
+            projectionFactory.entity().toProjection(),
+            projectionFactory.score().toProjection()


FWIW computing the score does incur additional cost in some cases, in particular when not sorting by score: https://github.com/hibernate/hibernate-search/blob/578ca56deb06d72da49a2b50d9cb0da437c192ed/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/search/extraction/impl/ExtractionRequirements.java#L81-L89, https://github.com/hibernate/hibernate-search/blob/7b6b15c04f0ce6ec98e8d4983605b97f36d34fd5/backend/lucene/src/main/java/org/hibernate/search/backend/lucene/search/extraction/impl/LuceneCollectors.java#L220-L231

Though I couldn't say how high the cost is exactly. "It depends", I guess :)

If it's easy, you might want to only do this when projecting on the score is actually required.

@yrodiere I wrongly thought that the score came for free, so I introduced it in all cases. Thanks for sharing it, I'm going to change the code to add it only if required by the current projections.

@yrodiere Fixed in the last commit. Thanks for catching it.

karesti · 2024-01-16T10:41:44Z

@fax4ever I'm going to build this PR and test it in langchain SPI

karesti · 2024-01-16T13:46:31Z

query/src/test/java/org/infinispan/query/projection/ScoreProjectionTest.java

+      query = cache.query("select g.name, score(g), g from org.infinispan.query.model.Game g where g.description : 'description'~2");
+      games = query.list();
+      assertThat(games).extracting(objects -> objects[0]).containsExactly("1", "2", "3", "4");
+      assertThat(games).extracting(objects -> objects[1]).hasOnlyElementsOfType(Float.class).isNotNull();


in langchain, the score is a double

only 4 test pass now

java.lang.AssertionError: Expecting actual: 0.7332834601402283 to be close to: 0.9090677379700546 by less than 1% but difference was 19.336763421211277%. (a difference of exactly 1% being considered valid) at dev.langchain4j.store.embedding.EmbeddingStoreWithoutMetadataIT.should_return_correct_score(EmbeddingStoreWithoutMetadataIT.java:237) at java.base/java.lang.reflect.Method.invoke(Method.java:568) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) [ERROR] dev.langchain4j.store.embedding.infinispan.InfinispanEmbeddingStoreIT.should_find_with_min_score -- Time elapsed: 0.155 s <<< FAILURE! java.lang.AssertionError: Expecting actual: 0.7332834601402283 to be close to: 0.9090677379700546 by less than 1% but difference was 19.336763421211277%. (a difference of exactly 1% being considered valid) at dev.langchain4j.store.embedding.EmbeddingStoreWithoutMetadataIT.should_find_with_min_score(EmbeddingStoreWithoutMetadataIT.java:188) at java.base/java.lang.reflect.Method.invoke(Method.java:568) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) [ERROR] dev.langchain4j.store.embedding.infinispan.InfinispanEmbeddingStoreIT.should_add_multiple_embeddings_with_segments -- Time elapsed: 0.152 s <<< FAILURE! java.lang.AssertionError: Expecting actual: 0.7332834601402283 to be close to: 0.9090677379700546 by less than 1% but difference was 19.336763421211277%. (a difference of exactly 1% being considered valid) at dev.langchain4j.store.embedding.EmbeddingStoreWithoutMetadataIT.should_add_multiple_embeddings_with_segments(EmbeddingStoreWithoutMetadataIT.java:160) at java.base/java.lang.reflect.Method.invoke(Method.java:568) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) [ERROR] dev.langchain4j.store.embedding.infinispan.InfinispanEmbeddingStoreIT.should_add_multiple_embeddings -- Time elapsed: 0.145 s <<< FAILURE! java.lang.AssertionError: Expecting actual: 0.7332834601402283 to be close to: 0.9090677379700546 by less than 1% but difference was 19.336763421211277%. (a difference of exactly 1% being considered valid) at dev.langchain4j.store.embedding.EmbeddingStoreWithoutMetadataIT.should_add_multiple_embeddings(EmbeddingStoreWithoutMetadataIT.java:121) at java.base/java.lang.reflect.Method.invoke(Method.java:568) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) [INFO] [INFO] Results: [INFO] [ERROR] Failures: [ERROR] InfinispanEmbeddingStoreIT>EmbeddingStoreWithoutMetadataIT.should_add_multiple_embeddings:121 Expecting actual: 0.7332834601402283 to be close to: 0.9090677379700546 by less than 1% but difference was 19.336763421211277%. (a difference of exactly 1% being considered valid) [ERROR] InfinispanEmbeddingStoreIT>EmbeddingStoreWithoutMetadataIT.should_add_multiple_embeddings_with_segments:160 Expecting actual: 0.7332834601402283 to be close to: 0.9090677379700546 by less than 1% but difference was 19.336763421211277%. (a difference of exactly 1% being considered valid) [ERROR] InfinispanEmbeddingStoreIT>EmbeddingStoreWithoutMetadataIT.should_find_with_min_score:188 Expecting actual: 0.7332834601402283 to be close to: 0.9090677379700546 by less than 1% but difference was 19.336763421211277%. (a difference of exactly 1% being considered valid) [ERROR] InfinispanEmbeddingStoreIT>EmbeddingStoreWithoutMetadataIT.should_return_correct_score:237 Expecting actual: 0.7332834601402283 to be close to: 0.9090677379700546 by less than 1% but difference was 19.336763421211277%. (a difference of exactly 1% being considered valid) [ERROR] Errors: [ERROR] InfinispanEmbeddingStoreIT>EmbeddingStoreWithoutMetadataIT.beforeEach:24->clearStore:45 » Transport io.netty.channel.AbstractChannel$AnnotatedConnectException: Connection refused: localhost/[0:0:0:0:0:0:0:1]:11222 [INFO] [ERROR] Tests run: 8, Failures: 4, Errors: 1, Skipped: 0 [INFO]

Hibernate Search returns a float, as Lucene does. I don't know if it makes sense to cast it to double from our side...

I don't know if it makes sense to cast it to double from our side...

I'm guessing Katia can do that in the Langchain4j SPI implementation?

Anyway, the problem here seems to be deeper: the test expects a certain value for the score, and that's not what it gets. No idea why... ? How was that expected score value determined?

@yrodiere IIUC we only need to change the distance function to cosine and after that all tests pass

karesti

test don't pass with the current implem

fax4ever · 2024-01-16T13:58:40Z

test don't pass with the current implem

Are you using Query<Object[]>? Let's check it on Zulip chat

fax4ever · 2024-01-16T14:11:17Z

@marko-bekhta @yrodiere thanks for the review I'm going to apply the changes

karesti

It worked, all test pass in langchain integration

fax4ever · 2024-01-16T15:58:17Z

@karesti thanks for checking it directly with Langchain!

Optimization / not strictly functional: This in the case of the score is not present neither in projection nor in sort

fax4ever · 2024-01-18T15:46:27Z

Thank you @tristantarrant

fax4ever force-pushed the ISPN-15390 branch 2 times, most recently from bd8f6bc to f33c09c Compare January 16, 2024 09:06

fax4ever marked this pull request as ready for review January 16, 2024 09:06

marko-bekhta reviewed Jan 16, 2024

View reviewed changes

yrodiere reviewed Jan 16, 2024

View reviewed changes

karesti reviewed Jan 16, 2024

View reviewed changes

karesti requested changes Jan 16, 2024

View reviewed changes

karesti approved these changes Jan 16, 2024

View reviewed changes

fax4ever force-pushed the ISPN-15390 branch from f33c09c to 8209676 Compare January 17, 2024 16:21

tristantarrant added this to the 15.0.0.Dev07 milestone Jan 17, 2024

fax4ever added 6 commits January 18, 2024 09:01

ISPN-15390 Use entry class for entity iterator

d2ca1fb

ISPN-15390 Parse score projections

b77927f

ISPN-15390 Support score projections

45884f4

ISPN-15390 Enforce the score on broadcast queries

b0baad5

ISPN-15390 Test broadcast/remote vector score projections

cd6167a

ISPN-15390 Avoid to project score if not required

72917de

Optimization / not strictly functional: This in the case of the score is not present neither in projection nor in sort

fax4ever force-pushed the ISPN-15390 branch from 8209676 to 72917de Compare January 18, 2024 08:40

yrodiere approved these changes Jan 18, 2024

View reviewed changes

tristantarrant merged commit bb07f62 into infinispan:main Jan 18, 2024
7 of 8 checks passed

fax4ever deleted the ISPN-15390 branch January 18, 2024 15:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ISPN-15390 Allow to project the query score #11679

ISPN-15390 Allow to project the query score #11679

fax4ever commented Jan 15, 2024 •

edited

fax4ever commented Jan 16, 2024

marko-bekhta left a comment

marko-bekhta Jan 16, 2024

fax4ever Jan 16, 2024

yrodiere Jan 16, 2024

fax4ever Jan 16, 2024

fax4ever Jan 18, 2024

karesti commented Jan 16, 2024

karesti Jan 16, 2024

karesti Jan 16, 2024

fax4ever Jan 16, 2024

yrodiere Jan 16, 2024 •

edited

fax4ever Jan 16, 2024

karesti left a comment

fax4ever commented Jan 16, 2024

fax4ever commented Jan 16, 2024

karesti left a comment

fax4ever commented Jan 16, 2024

fax4ever commented Jan 18, 2024

ISPN-15390 Allow to project the query score #11679

ISPN-15390 Allow to project the query score #11679

Conversation

fax4ever commented Jan 15, 2024 • edited

fax4ever commented Jan 16, 2024

marko-bekhta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karesti commented Jan 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yrodiere Jan 16, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karesti left a comment

Choose a reason for hiding this comment

fax4ever commented Jan 16, 2024

fax4ever commented Jan 16, 2024

karesti left a comment

Choose a reason for hiding this comment

fax4ever commented Jan 16, 2024

fax4ever commented Jan 18, 2024

fax4ever commented Jan 15, 2024 •

edited

yrodiere Jan 16, 2024 •

edited