Skip to content

Conversation

thecoop
Copy link
Member

@thecoop thecoop commented Oct 8, 2025

Add bfloat16 raw vector support to bbq_hnsw and bbq_disk vector types (but not enabled yet). Integration tests are part of #135940

@thecoop thecoop force-pushed the bbq_hnsw-bfloat16 branch 4 times, most recently from d305146 to 88b682c Compare October 10, 2025 09:34
double expected = Double.parseDouble(m.group(1));
double actual = Double.parseDouble(m.group(2));
double allowedError = expected * 0.01; // within 1%
assertThat(error.getMessage(), actual, closeTo(expected, allowedError));
Copy link
Member Author

@thecoop thecoop Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loss of precision causes several base vector tests in Lucene to hit assertions. This is one way to solve it - there are several others. This approach does mean that tests exit early and don't run to completion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loss of precision is OK, but yeah, its tricky to handle all the tests

@thecoop thecoop requested a review from benwtrent October 10, 2025 09:56
@thecoop thecoop marked this pull request as ready for review October 10, 2025 09:56
@elasticsearchmachine elasticsearchmachine added the Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch label Oct 10, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-relevance (Team:Search Relevance)

static final int DIRECT_MONOTONIC_BLOCK_SHIFT = 16;
private final FlatVectorsScorer vectorsScorer;

public ES93BFloat16FlatVectorsFormat(FlatVectorsScorer vectorsScorer) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should allow injectable scorers here.

We cannot use the getLucene99FlatVectorsScorer as it will assume vectors are float32, and attempt to score them off-heap if possible, which will completely break with bfloat16

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use the default vector scorers at the moment, as they work on FloatVectorValues, which has a bfloat16 implementation here. The panama scorer implementation is also ok, as that only works if HasIndexSlice is implemented, which it is not (now)

double expected = Double.parseDouble(m.group(1));
double actual = Double.parseDouble(m.group(2));
double allowedError = expected * 0.01; // within 1%
assertThat(error.getMessage(), actual, closeTo(expected, allowedError));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

loss of precision is OK, but yeah, its tricky to handle all the tests

@benwtrent
Copy link
Member

For my boolean vs element_type comments, I can understand keeping it boolean until we actually add bfloat16 support via API and enable it everywhere.

We just need to be sure not to forget about it.

@thecoop
Copy link
Member Author

thecoop commented Oct 10, 2025

I've turned the generic format into a proper format - it's a lot neater now

import java.util.Map;

public abstract class ES93GenericFlatVectorsFormat extends AbstractFlatVectorsFormat {
public class ES93GenericFlatVectorsFormat extends AbstractFlatVectorsFormat {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍

@thecoop thecoop requested a review from benwtrent October 13, 2025 13:59
@thecoop thecoop merged commit 7eaaff8 into elastic:main Oct 14, 2025
34 checks passed
@thecoop thecoop deleted the bbq_hnsw-bfloat16 branch October 14, 2025 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>non-issue :Search Relevance/Vectors Vector search Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v9.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants