You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ab...@apache.org on 2021/02/15 17:02:53 UTC
[lucene-solr] branch master updated: SOLR-15149: model creation
errors fixes (#2350)
This is an automated email from the ASF dual-hosted git repository.
abenedetti pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/master by this push:
new 227ef3b SOLR-15149: model creation errors fixes (#2350)
227ef3b is described below
commit 227ef3b39798d00c006539c9bee0e57db34c2871
Author: Alessandro Benedetti <a....@sease.io>
AuthorDate: Mon Feb 15 18:02:25 2021 +0100
SOLR-15149: model creation errors fixes (#2350)
SOLR-15149: model loading errors fix + tests
---
solr/CHANGES.txt | 2 +
.../org/apache/solr/ltr/model/LTRScoringModel.java | 2 +-
.../org/apache/solr/ltr/model/LinearModel.java | 4 +-
.../solr/ltr/store/rest/ManagedModelStore.java | 17 +++++---
.../linear-model_notExistentFeature.json | 19 +++++++++
.../linear-model_notExistentStore.json | 46 +++++++++++++++++++++
...ultipleadditivetreesmodel_notExistentStore.json | 39 ++++++++++++++++++
.../multipleadditivetreesmodel_unknownFeature.json | 38 +++++++++++++++++
.../org/apache/solr/ltr/model/TestLinearModel.java | 47 ++++++++++++++++++++++
.../ltr/model/TestMultipleAdditiveTreesModel.java | 25 ++++++++++++
10 files changed, 231 insertions(+), 8 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index c05cd84..b4592ed 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -239,6 +239,8 @@ Optimizations
Bug Fixes
---------------------
* SOLR-15078: Fix ExpandComponent behavior when expanding on numeric fields to differentiate '0' group from null group (hossman)
+* SOLR-15149: Better exception handling for LTR model creation errors (Alessandro Benedetti, Christine Poerschke)
+
Other Changes
---------------------
diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/model/LTRScoringModel.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/model/LTRScoringModel.java
index 9052ba1..e148759 100644
--- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/model/LTRScoringModel.java
+++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/model/LTRScoringModel.java
@@ -108,7 +108,7 @@ public abstract class LTRScoringModel implements Accountable {
SolrPluginUtils.invokeSetters(model, params.entrySet());
}
} catch (final Exception e) {
- throw new ModelException("Model type does not exist " + className, e);
+ throw new ModelException("Model loading failed for " + className, e);
}
model.validate();
return model;
diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/model/LinearModel.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/model/LinearModel.java
index e5b5064..965c0cb 100644
--- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/model/LinearModel.java
+++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/model/LinearModel.java
@@ -80,10 +80,10 @@ public class LinearModel extends LTRScoringModel {
public void setWeights(Object weights) {
@SuppressWarnings({"unchecked"})
- final Map<String,Double> modelWeights = (Map<String,Double>) weights;
+ final Map<String,Number> modelWeights = (Map<String, Number>) weights;
for (int ii = 0; ii < features.size(); ++ii) {
final String key = features.get(ii).getName();
- final Double val = modelWeights.get(key);
+ final Number val = modelWeights.get(key);
featureToWeight[ii] = (val == null ? null : val.floatValue());
}
}
diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/store/rest/ManagedModelStore.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/store/rest/ManagedModelStore.java
index add7021..0627d79 100644
--- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/store/rest/ManagedModelStore.java
+++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/store/rest/ManagedModelStore.java
@@ -294,11 +294,18 @@ public class ManagedModelStore extends ManagedResource implements ManagedResourc
return modelMap;
}
- private static Feature lookupFeatureFromFeatureMap(Map<String,Object> featureMap,
- FeatureStore featureStore) {
- final String featureName = (String)featureMap.get(NAME_KEY);
- return (featureName == null ? null
- : featureStore.get(featureName));
+ private static Feature lookupFeatureFromFeatureMap(Map<String, Object> featureMap, FeatureStore featureStore)
+ {
+ final String featureName = (String) featureMap.get(NAME_KEY);
+ Feature extractedFromStore = featureName == null ? null : featureStore.get(featureName);
+ if (extractedFromStore == null) {
+ if (featureStore.getFeatures().isEmpty()) {
+ throw new ModelException("Missing or empty feature store: " + featureStore.getName());
+ } else {
+ throw new ModelException("Feature: " + featureName + " not found in store: " + featureStore.getName());
+ }
+ }
+ return extractedFromStore;
}
@SuppressWarnings("unchecked")
diff --git a/solr/contrib/ltr/src/test-files/modelExamples/linear-model_notExistentFeature.json b/solr/contrib/ltr/src/test-files/modelExamples/linear-model_notExistentFeature.json
new file mode 100644
index 0000000..ffa2528
--- /dev/null
+++ b/solr/contrib/ltr/src/test-files/modelExamples/linear-model_notExistentFeature.json
@@ -0,0 +1,19 @@
+{
+ "class": "org.apache.solr.ltr.model.LinearModel",
+ "name": "6029760550880411648",
+ "store": "test",
+ "features": [
+ {
+ "name": "notExist1"
+ },
+ {
+ "name": "notExist2"
+ }
+ ],
+ "params": {
+ "weights": {
+ "notExist1": 0.0000000000,
+ "notExist2": 0.1000000000
+ }
+ }
+}
diff --git a/solr/contrib/ltr/src/test-files/modelExamples/linear-model_notExistentStore.json b/solr/contrib/ltr/src/test-files/modelExamples/linear-model_notExistentStore.json
new file mode 100644
index 0000000..fa651b3
--- /dev/null
+++ b/solr/contrib/ltr/src/test-files/modelExamples/linear-model_notExistentStore.json
@@ -0,0 +1,46 @@
+{
+ "class": "org.apache.solr.ltr.model.LinearModel",
+ "name": "6029760550880411648",
+ "store": "not_existent_store",
+ "features": [
+ {
+ "name": "title"
+ },
+ {
+ "name": "description"
+ },
+ {
+ "name": "keywords"
+ },
+ {
+ "name": "popularity",
+ "norm": {
+ "class": "org.apache.solr.ltr.norm.MinMaxNormalizer",
+ "params": {
+ "min": "0.0f",
+ "max": "10.0f"
+ }
+ }
+ },
+ {
+ "name": "text"
+ },
+ {
+ "name": "queryIntentPerson"
+ },
+ {
+ "name": "queryIntentCompany"
+ }
+ ],
+ "params": {
+ "weights": {
+ "title": 0.0000000000,
+ "description": 0.1000000000,
+ "keywords": 0.2000000000,
+ "popularity": 0.3000000000,
+ "text": 0.4000000000,
+ "queryIntentPerson": 0.1231231,
+ "queryIntentCompany": 0.12121211
+ }
+ }
+}
diff --git a/solr/contrib/ltr/src/test-files/modelExamples/multipleadditivetreesmodel_notExistentStore.json b/solr/contrib/ltr/src/test-files/modelExamples/multipleadditivetreesmodel_notExistentStore.json
new file mode 100644
index 0000000..4ec4938
--- /dev/null
+++ b/solr/contrib/ltr/src/test-files/modelExamples/multipleadditivetreesmodel_notExistentStore.json
@@ -0,0 +1,39 @@
+{
+ "class":"org.apache.solr.ltr.model.MultipleAdditiveTreesModel",
+ "name":"multipleadditivetreesmodel",
+ "store": "not_existent_store",
+ "features":[
+ { "name": "matchedTitle"},
+ { "name": "constantScoreToForceMultipleAdditiveTreesScoreAllDocs"}
+ ],
+ "params":{
+ "trees": [
+ {
+ "weight" : "1f",
+ "root": {
+ "feature": "matchedTitle",
+ "threshold": "0.5f",
+ "left" : {
+ "value" : "-100"
+ },
+ "right": {
+ "feature" : "constantScoreToForceMultipleAdditiveTreesScoreAllDocs",
+ "threshold": "10.0f",
+ "left" : {
+ "value" : "50"
+ },
+ "right" : {
+ "value" : "75"
+ }
+ }
+ }
+ },
+ {
+ "weight" : "2f",
+ "root": {
+ "value" : "-10"
+ }
+ }
+ ]
+ }
+}
diff --git a/solr/contrib/ltr/src/test-files/modelExamples/multipleadditivetreesmodel_unknownFeature.json b/solr/contrib/ltr/src/test-files/modelExamples/multipleadditivetreesmodel_unknownFeature.json
new file mode 100644
index 0000000..496d989
--- /dev/null
+++ b/solr/contrib/ltr/src/test-files/modelExamples/multipleadditivetreesmodel_unknownFeature.json
@@ -0,0 +1,38 @@
+{
+ "class":"org.apache.solr.ltr.model.MultipleAdditiveTreesModel",
+ "name":"multipleadditivetreesmodel",
+ "features":[
+ { "name": "notExist1"},
+ { "name": "notExist2"}
+ ],
+ "params":{
+ "trees": [
+ {
+ "weight" : "1f",
+ "root": {
+ "feature": "notExist1",
+ "threshold": "0.5f",
+ "left" : {
+ "value" : "-100"
+ },
+ "right": {
+ "feature" : "notExist2",
+ "threshold": "10.0f",
+ "left" : {
+ "value" : "50"
+ },
+ "right" : {
+ "value" : "75"
+ }
+ }
+ }
+ },
+ {
+ "weight" : "2f",
+ "root": {
+ "value" : "-10"
+ }
+ }
+ ]
+ }
+}
diff --git a/solr/contrib/ltr/src/test/org/apache/solr/ltr/model/TestLinearModel.java b/solr/contrib/ltr/src/test/org/apache/solr/ltr/model/TestLinearModel.java
index d5950e6..91e033b 100644
--- a/solr/contrib/ltr/src/test/org/apache/solr/ltr/model/TestLinearModel.java
+++ b/solr/contrib/ltr/src/test/org/apache/solr/ltr/model/TestLinearModel.java
@@ -185,6 +185,29 @@ public class TestLinearModel extends TestRerankBase {
}
@Test
+ public void integerAndLongFeatureWeights_shouldCreateModel() {
+ final List<Feature> features = getFeatures(new String[]
+ {"constant1", "constant5"});
+ final List<Normalizer> norms =
+ new ArrayList<>(Collections.nCopies(features.size(),IdentityNormalizer.INSTANCE));
+
+ final Map<String,Object> weights = new HashMap<>();
+ weights.put("constant1", 100L);
+ weights.put("constant5", 1);
+
+ Map<String,Object> params = new HashMap<>();
+ params.put("weights", weights);
+
+ final LTRScoringModel ltrScoringModel = createLinearModel("test6",
+ features, norms, "test", fstore.getFeatures(),
+ params);
+
+ store.addModel(ltrScoringModel);
+ final LTRScoringModel m = store.getModel("test6");
+ assertEquals(ltrScoringModel, m);
+ }
+
+ @Test
public void emptyFeaturesTest() {
final ModelException expectedException =
new ModelException("no features declared for model test6");
@@ -206,4 +229,28 @@ public class TestLinearModel extends TestRerankBase {
assertEquals(expectedException.toString(), ex.toString());
}
+ @Test
+ public void notExistentStore_shouldThrowMeaningFulException(){
+ final ModelException expectedException =
+ new ModelException("Feature Store not found: not_existent_store");
+
+ ModelException ex = expectThrows(ModelException.class, () -> {
+ createModelFromFiles("linear-model_notExistentStore.json",
+ "features-store-test-model.json");
+ });
+ assertEquals(expectedException.toString(), ex.toString());
+ }
+
+ @Test
+ public void notExistentFeature_shouldThrowMeaningFulException(){
+ final ModelException expectedException =
+ new ModelException("Feature:notExist1 not found in store: test");
+
+ ModelException ex = expectThrows(ModelException.class, () -> {
+ createModelFromFiles("linear-model_notExistentFeature.json",
+ "features-store-test-model.json");
+ });
+ assertEquals(expectedException.toString(), ex.toString());
+ }
+
}
diff --git a/solr/contrib/ltr/src/test/org/apache/solr/ltr/model/TestMultipleAdditiveTreesModel.java b/solr/contrib/ltr/src/test/org/apache/solr/ltr/model/TestMultipleAdditiveTreesModel.java
index 8cb59f2..1cf2798 100644
--- a/solr/contrib/ltr/src/test/org/apache/solr/ltr/model/TestMultipleAdditiveTreesModel.java
+++ b/solr/contrib/ltr/src/test/org/apache/solr/ltr/model/TestMultipleAdditiveTreesModel.java
@@ -225,4 +225,29 @@ public class TestMultipleAdditiveTreesModel extends TestRerankBase {
});
assertEquals(expectedException.toString(), ex.toString());
}
+
+ @Test
+ public void multipleAdditiveTreesTestMissingFeatureStore(){
+ final ModelException expectedException =
+ new ModelException("Feature Store not found: not_existent_store");
+
+ ModelException ex = expectThrows(ModelException.class, () -> {
+ createModelFromFiles("multipleadditivetreesmodel_notExistentStore.json",
+ "multipleadditivetreesmodel_features.json");
+ });
+ assertEquals(expectedException.toString(), ex.toString());
+ }
+
+ @Test
+ public void multipleAdditiveTreesTestUnknownFeature(){
+ final ModelException expectedException =
+ new ModelException("Feature:notExist1 not found in store: _DEFAULT_");
+
+ ModelException ex = expectThrows(ModelException.class, () -> {
+ createModelFromFiles("multipleadditivetreesmodel_unknownFeature.json",
+ "multipleadditivetreesmodel_features.json");
+ });
+ assertEquals(expectedException.toString(), ex.toString());
+ }
+
}