Parameterize VectorSimilarityFunctionsTests#139516
Conversation
thecoop
added
>test
elasticsearchmachine
added
v9.3.0
Team:Search Relevance
|
Pinging @elastic/es-search-relevance (Team:Search Relevance) |
thecoop
force-pushed
the
parameterized-jdkvectorlibrary-tests
branch
from
2d9fc82 to
f51c0f6
Compare
| // trivial bulk with a single vector | ||
| float[] bulkScore = new float[1]; | ||
| dotProduct7uBulk(nativeSeg1, nativeSeg2, dims, 1, MemorySegment.ofArray(bulkScore)); | ||
| similarityBulk(nativeSeg1, nativeSeg2, dims, 1, MemorySegment.ofArray(bulkScore)); |
There was a problem hiding this comment.
This will currently throw an AssumptionViolatedException with square distance, which will mark the whole test as ignored, but that'll soon be fixed. If there are errors in the single square distance function, those will fail before getting here, and so will fail the test outright.
ldematte
left a comment
ldematte
left a comment
There was a problem hiding this comment.
LGTM! Nice and clean way to have a complete matrix of tests, thanks!
libs/native/src/test/java/org/elasticsearch/nativeaccess/jdk/JDKVectorLibraryInt7uTests.java
Show resolved
Hide resolved
| public static final Class<IllegalArgumentException> IAE = IllegalArgumentException.class; | ||
| public static final Class<IndexOutOfBoundsException> IOOBE = IndexOutOfBoundsException.class; | ||
|
|
||
| public enum SimilarityFunction { |
There was a problem hiding this comment.
What do you think about using simdvec VectorSimilarityFunction or lucene VectorSimilarityType here, so we don't need to keep all in sync?
There was a problem hiding this comment.
The simdvec one is not accessible here. We could use the Lucene one, but that has MAXIMUM_INNER_PRODUCT which is just a variant of DOT_PRODUCT that doesnt affect the simd implementation.
There was a problem hiding this comment.
My only worry here was to keep it as foolproof as possible, so if we add anything things will automatically break very early and we'll know what we need to add.
There was a problem hiding this comment.
But it's a minor nit, I'm good as it is
There was a problem hiding this comment.
I'll look at uses when both this and #139423 are merged
…-err-message * upstream/main: (45 commits) Add sort field usage to telemetry (elastic#139530) ES|QL: Release CCS support for FORK (elastic#139630) Parameterize VectorSimilarityFunctionsTests on the similarity function (elastic#139516) fix broken links from ccs file move (elastic#139655) Update docs for v9.2.3 release (elastic#139479) Move tsdb bwc tests to x-pack/logsdb (elastic#139671) Fix FirstDocIdGroupingAggregatorFunction (elastic#139619) Fix release test for node_reduction profiling (elastic#139515) Unmute RestClientSingleHostIntegTests.testRequestResetAndAbort (elastic#139656) Unmute VerifyVersionConstantsIT.testLuceneVersionConstant (elastic#139644) Mute org.elasticsearch.repositories.gcs.GoogleCloudStorageBlobStoreRepositoryTests testReadNonExistingPath elastic#139665 Mute org.elasticsearch.smoketest.WatcherYamlRestIT test {p0=mustache/10_webhook/Test webhook action with mustache integration} elastic#139663 [ES|QL] Run aggregations on aggregate metric double with default metric (elastic#138647) Enable TDigest field mapper and ES|QL type (elastic#139607) Mute org.elasticsearch.index.mapper.HalfFloatSyntheticSourceNativeArrayIntegrationTests testSynthesizeArrayRandom elastic#139658 Explicitly cleanup test index with shared data path (elastic#136048) Mute org.elasticsearch.xpack.core.action.XPackUsageResponseTests testVersionDependentSerializationWriteToOldStream elastic#139576 Make XPackUsageResponseTests a wire serializing test case (elastic#139643) Mute org.elasticsearch.upgrades.UpgradeClusterClientYamlTestSuiteIT test {p0=mixed_cluster/90_ml_data_frame_analytics_crud/Start and stop old regression job} elastic#139654 Relax error bounds for RandomizedTimeSeriesIT (elastic#139641) ...
3 participants
Parameterize the tests on the similarity function, allowing more to be added easily