Repository navigation
Update semantic text to use BFLOAT16 by default - #144236
Conversation
…lement type, dimensions, and similarity match what is expected
|
Hi @Mikep86, I've created a changelog YAML for you. |
|
@elasticmachine update branch |
|
I found a couple bugs while working on this: Cannot get semantic_text field mapping with defaults when inference service does not exist - Fixed this for 9.4 in this PR, since I was already modifying the code that would need to change anyways. It should be easy to apply the necessary changes to earlier branches via a manual backport. Unhandled edge cases when setting semantic_text index options - I left the failure modes the same so we can address this in a separate PR. |
| if (includeDefaults || isConfigured()) { | ||
| if (value == null) { | ||
| // Default value, serialize resolved defaults | ||
| MinimalServiceSettings resolvedModelSettings = getResolvedModelSettings(null, false); |
There was a problem hiding this comment.
This is the fix for #145136. getResolvedModelSettings must return null when the inference service can't be found.
| if (resolved != null && settings.canMergeWith(resolved) == false) { | ||
| throw new IllegalArgumentException( | ||
| "Mismatch between provided and registered inference model settings. " | ||
| + "Provided: [" | ||
| + settings | ||
| + "], Expected: [" | ||
| + resolved | ||
| + "]." | ||
| + modelSettings.taskType().name() | ||
| ); |
There was a problem hiding this comment.
@jimczi I removed this because this check was ineffective. It was only ever called when both settings and resolved the same instance, set via modelSettings.get().
| public void testGetDefaultIndexOptionsBeforeInferenceServiceExists() throws Exception { | ||
| final String inferenceId = randomIdentifier(); | ||
| final String inferenceFieldName = "inference_field"; | ||
|
|
||
| // Create the index before the inference endpoint exists. Default index options cannot be determined yet. | ||
| assertAcked(safeGet(prepareCreate(INDEX_NAME).setMapping(generateMapping(inferenceFieldName, inferenceId, null)).execute())); | ||
| Map<String, Object> actualFieldMappings = getFieldMappings(inferenceFieldName, true); | ||
|
|
||
| Map<String, Object> inferenceFieldMappings = XContentMapValues.nodeMapValue( | ||
| actualFieldMappings.get(inferenceFieldName), | ||
| inferenceFieldName | ||
| ); | ||
| assertThat(inferenceFieldMappings.containsKey("index_options"), is(true)); | ||
| assertThat(inferenceFieldMappings.get("index_options"), nullValue()); |
There was a problem hiding this comment.
This is the test that validates the fix for #145136
|
Pinging @elastic/es-search-relevance (Team:Search Relevance) |
|
Pinging @elastic/es-search-foundations (Team:Search Foundations) |
kkharbas
left a comment
There was a problem hiding this comment.
LGTM! Except one nit comment
Updates the semantic_text field to use the BFLOAT16 element type by default for all inference services that use FLOAT. This change minorly impacts knn query scoring due to the reduced precision of BFLOAT16. This won't matter for 99% of users, but since this may be a problem for some, this PR also adds a way to override the element type used so that these users can continue using FLOAT. An element_type parameter has been added to the dense vector index options.
Updates the
semantic_textfield to use theBFLOAT16element type by default for all inference services that useFLOAT.This change minorly impacts
knnquery scoring due to the reduced precision ofBFLOAT16. This won't matter for 99% of users, but since this may be a problem for some, this PR also adds a way to override the element type used so that these users can continue usingFLOAT. Anelement_typeparameter has been added to the dense vector index options:Since
element_typeis not part of standard dense vector index options (and it doesn't make sense to add it there, sinceelement_typeis a top-level param fordense_vector), this param was added by extending dense vector index options only forsemantic_text. This creates some new scenarios for us to handle:element_typewithout settingtypetypeonly when params other thanelement_typeare setelement_typeset inindex_optionsis incompatible with the model element typeelement_typewheninclude_defaultsis true and we defaulted toBFLOAT16