From af05d3824739f9f1965c5e8da80fbe15b3d47c0a Mon Sep 17 00:00:00 2001 From: Aurelien FOUCRET Date: Fri, 4 Apr 2025 09:55:16 +0200 Subject: [PATCH 1/7] Updating YAML Rest tests for LTR --- .../test/ml/learning_to_rank_rescorer.yml | 36 +++++++++---------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/ml/learning_to_rank_rescorer.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/ml/learning_to_rank_rescorer.yml index 5c0096e9666fc..2f849e2bcd74a 100644 --- a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/ml/learning_to_rank_rescorer.yml +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/ml/learning_to_rank_rescorer.yml @@ -149,9 +149,6 @@ setup: --- "Test rescore with stored model": - - skip: - awaits_fix: "https://github.com/elastic/elasticsearch/issues/80703" - - do: search: index: store @@ -182,12 +179,11 @@ setup: - match: { hits.hits.0._score: 6.0 } - match: { hits.hits.1._score: 6.0 } - match: { hits.hits.2._score: 3.0 } + --- "Test rescore with stored model and smaller window_size": - - skip: - awaits_fix: "https://github.com/elastic/elasticsearch/issues/80703" - - do: + catch: bad_request search: index: store size: 5 @@ -198,16 +194,12 @@ setup: "learning_to_rank": { "model_id": "ltr-model" } } } - - match: { hits.hits.0._score: 17.0 } - - match: { hits.hits.1._score: 17.0 } - - match: { hits.hits.2._score: 1.0 } - - match: { hits.hits.3._score: 1.0 } - - match: { hits.hits.4._score: 1.0 } + - match: { status: 400 } + - match: { error.root_cause.0.type: "action_request_validation_exception" } + - match: { error.root_cause.0.reason: "Validation Failed: 1: rescorer [window_size] is too small and should be at least the value of [from + size: 4] but was [2];" } + --- "Test rescore with stored model and chained rescorers": - - skip: - awaits_fix: "https://github.com/elastic/elasticsearch/issues/80703" - - do: search: index: store @@ -216,11 +208,11 @@ setup: { "rescore": [ { - "window_size": 4, + "window_size": 10, "query": { "rescore_query":{ "script_score": {"query": {"match_all": {}}, "script": {"source": "return 4"}}}} }, { - "window_size": 3, + "window_size": 5, "learning_to_rank": { "model_id": "ltr-model" } }, { @@ -232,8 +224,9 @@ setup: - match: { hits.hits.0._score: 37.0 } - match: { hits.hits.1._score: 37.0 } - match: { hits.hits.2._score: 14.0 } - - match: { hits.hits.3._score: 5.0 } - - match: { hits.hits.4._score: 1.0 } + - match: { hits.hits.3._score: 6.0 } + - match: { hits.hits.4._score: 6.0 } + --- "Test rescore with missing model": - do: @@ -247,8 +240,12 @@ setup: "learning_to_rank": { "model_id": "ltr-missing" } } } + - match: { status: 404 } + - match: { error.root_cause.0.type: "resource_not_found_exception" } + - match: { error.root_cause.0.reason: "Could not find trained model [ltr-missing]" } + --- -"Test rescore with no hits model": +"Test rescore with no hits": - do: search: index: store @@ -261,6 +258,7 @@ setup: } } - length: { hits.hits: 0 } + --- "Test model input validation": - skip: From 4e8cfcc7b33c8a71c32d77a7f1586e628f43fbfd Mon Sep 17 00:00:00 2001 From: Aurelien FOUCRET Date: Fri, 4 Apr 2025 10:25:34 +0200 Subject: [PATCH 2/7] Adding a test case for model alias --- .../test/ml/learning_to_rank_rescorer.yml | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/ml/learning_to_rank_rescorer.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/ml/learning_to_rank_rescorer.yml index 2f849e2bcd74a..8d6d0a461b67e 100644 --- a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/ml/learning_to_rank_rescorer.yml +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/ml/learning_to_rank_rescorer.yml @@ -259,6 +259,31 @@ setup: } - length: { hits.hits: 0 } +--- +"Test rescore using model alias": + - skip: + features: headers + - do: + headers: + Authorization: "Basic eF9wYWNrX3Jlc3RfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" # run as x_pack_rest_user, i.e. the test setup superuser + ml.put_trained_model_alias: + model_id: "ltr-model" + model_alias: "ltr-model-alias" + - do: + search: + index: store + size: 3 + body: > + { + "rescore": { + "window_size": 10, + "learning_to_rank": { "model_id": "ltr-model-alias" } + } + } + - match: { hits.hits.0._score: 17.0 } + - match: { hits.hits.1._score: 17.0 } + - match: { hits.hits.2._score: 14.0 } + --- "Test model input validation": - skip: From 2702ebbd0b5925a30608e0c7e3af9b241aa8ab5b Mon Sep 17 00:00:00 2001 From: Aurelien FOUCRET Date: Fri, 4 Apr 2025 10:57:13 +0200 Subject: [PATCH 3/7] Ensure the model alias is translated to a model id. --- .../xpack/ml/inference/loadingservice/ModelLoadingService.java | 3 +-- .../xpack/ml/inference/ltr/LearningToRankService.java | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/loadingservice/ModelLoadingService.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/loadingservice/ModelLoadingService.java index a4646bb4a2168..8a001fd56814e 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/loadingservice/ModelLoadingService.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/loadingservice/ModelLoadingService.java @@ -239,8 +239,7 @@ public ModelLoadingService( this.licenseState = licenseState; } - // for testing - String getModelId(String modelIdOrAlias) { + public String getModelId(String modelIdOrAlias) { return modelAliasToId.getOrDefault(modelIdOrAlias, modelIdOrAlias); } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/ltr/LearningToRankService.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/ltr/LearningToRankService.java index bec162d141eba..977140b8695a1 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/ltr/LearningToRankService.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/ltr/LearningToRankService.java @@ -101,7 +101,7 @@ public void loadLocalModel(String modelId, ActionListener listener) */ public void loadLearningToRankConfig(String modelId, Map params, ActionListener listener) { trainedModelProvider.getTrainedModel( - modelId, + modelLoadingService.getModelId(modelId), GetTrainedModelsAction.Includes.all(), null, ActionListener.wrap(trainedModelConfig -> { From 565036c93c2c5e0adf3bcefa0bc232c5ea651817 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20FOUCRET?= Date: Fri, 4 Apr 2025 11:07:38 +0200 Subject: [PATCH 4/7] Update docs/changelog/126273.yaml --- docs/changelog/126273.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/126273.yaml diff --git a/docs/changelog/126273.yaml b/docs/changelog/126273.yaml new file mode 100644 index 0000000000000..420c0eb317a03 --- /dev/null +++ b/docs/changelog/126273.yaml @@ -0,0 +1,5 @@ +pr: 126273 +summary: Fix LTR rescorer with model alias +area: Ranking +type: bug +issues: [] From c3f3792f5be7c9d2e7ed6a7a42292d5446e6e76d Mon Sep 17 00:00:00 2001 From: Aurelien FOUCRET Date: Fri, 4 Apr 2025 11:24:36 +0200 Subject: [PATCH 5/7] Add missing stubbed method in LearningToRankServiceTests --- .../xpack/ml/inference/ltr/LearningToRankServiceTests.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/ltr/LearningToRankServiceTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/ltr/LearningToRankServiceTests.java index 46e54ff3f8c3d..e374517f6e82c 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/ltr/LearningToRankServiceTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/ltr/LearningToRankServiceTests.java @@ -42,12 +42,14 @@ import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.hasSize; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isA; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; public class LearningToRankServiceTests extends ESTestCase { public static final String GOOD_MODEL = "inference-entity-id"; @@ -185,7 +187,10 @@ protected NamedXContentRegistry xContentRegistry() { } private ModelLoadingService mockModelLoadingService() { - return mock(ModelLoadingService.class); + ModelLoadingService modelLoadingService = mock(ModelLoadingService.class); + when(modelLoadingService.getModelId(anyString())).thenAnswer(i -> i.getArgument(0)); + + return modelLoadingService; } @SuppressWarnings("unchecked") From 01ecc176e4b4ab2d325c018182ba7db89b416248 Mon Sep 17 00:00:00 2001 From: Aurelien FOUCRET Date: Fri, 4 Apr 2025 13:41:24 +0200 Subject: [PATCH 6/7] Move model alias creation during test setup. --- .../test/ml/learning_to_rank_rescorer.yml | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/ml/learning_to_rank_rescorer.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/ml/learning_to_rank_rescorer.yml index 8d6d0a461b67e..4208ee5bee4b8 100644 --- a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/ml/learning_to_rank_rescorer.yml +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/ml/learning_to_rank_rescorer.yml @@ -147,6 +147,13 @@ setup: { "index": {} } { "product": "Laptop", "cost": 500} + - do: + headers: + Authorization: "Basic eF9wYWNrX3Jlc3RfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" # run as x_pack_rest_user, i.e. the test setup superuser + ml.put_trained_model_alias: + model_id: "ltr-model" + model_alias: "ltr-model-alias" + --- "Test rescore with stored model": - do: @@ -263,12 +270,6 @@ setup: "Test rescore using model alias": - skip: features: headers - - do: - headers: - Authorization: "Basic eF9wYWNrX3Jlc3RfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" # run as x_pack_rest_user, i.e. the test setup superuser - ml.put_trained_model_alias: - model_id: "ltr-model" - model_alias: "ltr-model-alias" - do: search: index: store From c157d2b190b763998d0af1812c0e0e2c5d56fa4e Mon Sep 17 00:00:00 2001 From: Aurelien FOUCRET Date: Thu, 10 Apr 2025 17:07:58 +0200 Subject: [PATCH 7/7] Removed flaky tests (already covered by Java rest test). --- .../test/ml/learning_to_rank_rescorer.yml | 349 ------------------ 1 file changed, 349 deletions(-) delete mode 100644 x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/ml/learning_to_rank_rescorer.yml diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/ml/learning_to_rank_rescorer.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/ml/learning_to_rank_rescorer.yml deleted file mode 100644 index 4208ee5bee4b8..0000000000000 --- a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/ml/learning_to_rank_rescorer.yml +++ /dev/null @@ -1,349 +0,0 @@ -setup: - - skip: - features: headers - - do: - headers: - Authorization: "Basic eF9wYWNrX3Jlc3RfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" # run as x_pack_rest_user, i.e. the test setup superuser - ml.put_trained_model: - model_id: ltr-model - body: > - { - "description": "super complex model for tests", - "inference_config": { - "learning_to_rank": { - "feature_extractors": [ - { - "query_extractor": { - "feature_name": "cost", - "query": {"script_score": {"query": {"match_all":{}}, "script": {"source": "return doc['cost'].value;"}}} - } - }, - { - "query_extractor": { - "feature_name": "type_tv", - "query": {"term": {"product": "TV"}} - } - }, - { - "query_extractor": { - "feature_name": "type_vcr", - "query": {"term": {"product": "VCR"}} - } - }, - { - "query_extractor": { - "feature_name": "type_laptop", - "query": {"term": {"product": "Laptop"}} - } - } - ] - } - }, - "definition": { - "trained_model": { - "ensemble": { - "feature_names": ["cost", "type_tv", "type_vcr", "type_laptop"], - "target_type": "regression", - "trained_models": [ - { - "tree": { - "feature_names": [ - "cost" - ], - "tree_structure": [ - { - "node_index": 0, - "split_feature": 0, - "split_gain": 12, - "threshold": 400, - "decision_type": "lte", - "default_left": true, - "left_child": 1, - "right_child": 2 - }, - { - "node_index": 1, - "leaf_value": 5.0 - }, - { - "node_index": 2, - "leaf_value": 2.0 - } - ], - "target_type": "regression" - } - }, - { - "tree": { - "feature_names": [ - "type_tv" - ], - "tree_structure": [ - { - "node_index": 0, - "split_feature": 0, - "split_gain": 12, - "threshold": 1, - "decision_type": "lt", - "default_left": true, - "left_child": 1, - "right_child": 2 - }, - { - "node_index": 1, - "leaf_value": 1.0 - }, - { - "node_index": 2, - "leaf_value": 12.0 - } - ], - "target_type": "regression" - } - } - ] - } - } - } - } - - - do: - headers: - Authorization: "Basic eF9wYWNrX3Jlc3RfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" # run as x_pack_rest_user, i.e. the test setup superuser - indices.create: - index: store - body: - mappings: - properties: - product: - type: keyword - cost: - type: integer - - - do: - headers: - Authorization: "Basic eF9wYWNrX3Jlc3RfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" # run as x_pack_rest_user, i.e. the test setup superuser - Content-Type: application/json - bulk: - index: store - refresh: true - body: | - { "index": {} } - { "product": "TV", "cost": 300 } - { "index": {} } - { "product": "TV", "cost": 400} - { "index": {} } - { "product": "TV", "cost": 600} - { "index": {} } - { "product": "VCR", "cost": 15} - { "index": {} } - { "product": "VCR", "cost": 350} - { "index": {} } - { "product": "VCR", "cost": 580} - { "index": {} } - { "product": "Laptop", "cost": 100} - { "index": {} } - { "product": "Laptop", "cost": 300} - { "index": {} } - { "product": "Laptop", "cost": 500} - - - do: - headers: - Authorization: "Basic eF9wYWNrX3Jlc3RfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" # run as x_pack_rest_user, i.e. the test setup superuser - ml.put_trained_model_alias: - model_id: "ltr-model" - model_alias: "ltr-model-alias" - ---- -"Test rescore with stored model": - - do: - search: - index: store - size: 3 - body: > - { - "rescore": { - "window_size": 10, - "learning_to_rank": { "model_id": "ltr-model" } - } - } - - match: { hits.hits.0._score: 17.0 } - - match: { hits.hits.1._score: 17.0 } - - match: { hits.hits.2._score: 14.0 } - - - do: - search: - index: store - size: 3 - body: > - { - "query": {"term": {"product": "Laptop"}}, - "rescore": { - "window_size": 10, - "learning_to_rank": { "model_id": "ltr-model" } - } - } - - match: { hits.hits.0._score: 6.0 } - - match: { hits.hits.1._score: 6.0 } - - match: { hits.hits.2._score: 3.0 } - ---- -"Test rescore with stored model and smaller window_size": - - do: - catch: bad_request - search: - index: store - size: 5 - body: > - { - "rescore": { - "window_size": 2, - "learning_to_rank": { "model_id": "ltr-model" } - } - } - - match: { status: 400 } - - match: { error.root_cause.0.type: "action_request_validation_exception" } - - match: { error.root_cause.0.reason: "Validation Failed: 1: rescorer [window_size] is too small and should be at least the value of [from + size: 4] but was [2];" } - ---- -"Test rescore with stored model and chained rescorers": - - do: - search: - index: store - size: 5 - body: > - { - "rescore": [ - { - "window_size": 10, - "query": { "rescore_query":{ "script_score": {"query": {"match_all": {}}, "script": {"source": "return 4"}}}} - }, - { - "window_size": 5, - "learning_to_rank": { "model_id": "ltr-model" } - }, - { - "window_size": 2, - "query": { "rescore_query": { "script_score": {"query": {"match_all": {}}, "script": {"source": "return 20"}}}} - } - ] - } - - match: { hits.hits.0._score: 37.0 } - - match: { hits.hits.1._score: 37.0 } - - match: { hits.hits.2._score: 14.0 } - - match: { hits.hits.3._score: 6.0 } - - match: { hits.hits.4._score: 6.0 } - ---- -"Test rescore with missing model": - - do: - catch: missing - search: - index: store - body: > - { - "rescore": { - "window_size": 10, - "learning_to_rank": { "model_id": "ltr-missing" } - } - } - - match: { status: 404 } - - match: { error.root_cause.0.type: "resource_not_found_exception" } - - match: { error.root_cause.0.reason: "Could not find trained model [ltr-missing]" } - ---- -"Test rescore with no hits": - - do: - search: - index: store - body: > - { - "query": {"term": {"product": "Speaker"}}, - "rescore": { - "window_size": 10, - "learning_to_rank": { "model_id": "ltr-model" } - } - } - - length: { hits.hits: 0 } - ---- -"Test rescore using model alias": - - skip: - features: headers - - do: - search: - index: store - size: 3 - body: > - { - "rescore": { - "window_size": 10, - "learning_to_rank": { "model_id": "ltr-model-alias" } - } - } - - match: { hits.hits.0._score: 17.0 } - - match: { hits.hits.1._score: 17.0 } - - match: { hits.hits.2._score: 14.0 } - ---- -"Test model input validation": - - skip: - features: headers - - do: - headers: - Authorization: "Basic eF9wYWNrX3Jlc3RfdXNlcjp4LXBhY2stdGVzdC1wYXNzd29yZA==" # run as x_pack_rest_user, i.e. the test setup superuser - catch: bad_request - ml.put_trained_model: - model_id: bad-model - body: > - { - "description": "a bad model", - "input": { - "field_names": ["cost"] - }, - "inference_config": { - "learning_to_rank": { } - }, - "definition": { - "trained_model": { - "ensemble": { - "feature_names": ["cost"], - "target_type": "regression", - "trained_models": [ - { - "tree": { - "feature_names": [ - "cost" - ], - "tree_structure": [ - { - "node_index": 0, - "split_feature": 0, - "split_gain": 12, - "threshold": 400, - "decision_type": "lte", - "default_left": true, - "left_child": 1, - "right_child": 2 - }, - { - "node_index": 1, - "leaf_value": 5.0 - }, - { - "node_index": 2, - "leaf_value": 2.0 - } - ], - "target_type": "regression" - } - } - ] - } - } - } - } - - - match: { status: 400 } - - match: { error.root_cause.0.type: "action_request_validation_exception" } - - match: { error.root_cause.0.reason: "Validation Failed: 1: cannot specify [input.field_names] for a model of type [learning_to_rank];" }