-
Notifications
You must be signed in to change notification settings - Fork 25.6k
[EIS] Rename the elser 2 default model and the default inference endpoint #130336
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
abe5318
c1da4e0
3dd9081
3aebb35
566cc7c
f2c202a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
pr: 130336 | ||
summary: "[EIS] Rename the elser 2 default model and the default inference endpoint" | ||
area: Machine Learning | ||
type: bug | ||
issues: [] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -106,9 +106,9 @@ public class ElasticInferenceService extends SenderService { | |
static final String DEFAULT_CHAT_COMPLETION_MODEL_ID_V1 = "rainbow-sprinkles"; | ||
static final String DEFAULT_CHAT_COMPLETION_ENDPOINT_ID_V1 = defaultEndpointId(DEFAULT_CHAT_COMPLETION_MODEL_ID_V1); | ||
|
||
// elser-v2 | ||
static final String DEFAULT_ELSER_MODEL_ID_V2 = "elser-v2"; | ||
static final String DEFAULT_ELSER_ENDPOINT_ID_V2 = defaultEndpointId(DEFAULT_ELSER_MODEL_ID_V2); | ||
// elser-2 | ||
static final String DEFAULT_ELSER_2_MODEL_ID = "elser_model_2"; | ||
static final String DEFAULT_ELSER_ENDPOINT_ID_V2 = defaultEndpointId("elser-2"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't love this mismatch, but I imagine there are some considerations you're keeping in mind. Does changing it have backwards compatibility ramifications? Trying to think what the impact is, but can't figure it out 😅 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm with you, it doesn't look great. But the goal here is to be consistent with how things are called in the ML node, at least for ELSER. The model is called .elser_model_2 and the inference endpoint is Now, the name of the reranker is .rerank-v1. That's how we got here... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
These endpoints are not released yet, so we have the luxury of being able to change the name still. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ahh, gotcha. Thanks for the clarification! |
||
|
||
// multilingual-text-embed | ||
static final String DEFAULT_MULTILINGUAL_EMBED_MODEL_ID = "multilingual-embed-v1"; | ||
|
@@ -174,13 +174,13 @@ private static Map<String, DefaultModelConfig> initDefaultEndpoints( | |
), | ||
MinimalServiceSettings.chatCompletion(NAME) | ||
), | ||
DEFAULT_ELSER_MODEL_ID_V2, | ||
DEFAULT_ELSER_2_MODEL_ID, | ||
new DefaultModelConfig( | ||
new ElasticInferenceServiceSparseEmbeddingsModel( | ||
DEFAULT_ELSER_ENDPOINT_ID_V2, | ||
TaskType.SPARSE_EMBEDDING, | ||
NAME, | ||
new ElasticInferenceServiceSparseEmbeddingsServiceSettings(DEFAULT_ELSER_MODEL_ID_V2, null, null), | ||
new ElasticInferenceServiceSparseEmbeddingsServiceSettings(DEFAULT_ELSER_2_MODEL_ID, null, null), | ||
EmptyTaskSettings.INSTANCE, | ||
EmptySecretSettings.INSTANCE, | ||
elasticInferenceServiceComponents, | ||
|
@@ -213,7 +213,6 @@ private static Map<String, DefaultModelConfig> initDefaultEndpoints( | |
DenseVectorFieldMapper.ElementType.FLOAT | ||
) | ||
), | ||
|
||
DEFAULT_RERANK_MODEL_ID_V1, | ||
new DefaultModelConfig( | ||
new ElasticInferenceServiceRerankModel( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to remove
v
from the text embedding and rerank default endpoint names too?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, we'll only do the adjustment for ELSER to be en par with the ML node