From c297180cca19eff3f9ace04ef8e56e8418151bd0 Mon Sep 17 00:00:00 2001 From: Christine Poerschke Date: Thu, 3 Aug 2017 16:07:40 +0100 Subject: [PATCH 1/2] SOLR-11163: Fix contrib/ltr Normalizer persistence after solr core reload or restart. (Yuki Yano via Christine Poerschke) --- solr/CHANGES.txt | 3 +++ .../apache/solr/ltr/norm/MinMaxNormalizer.java | 4 ++-- .../solr/ltr/norm/StandardNormalizer.java | 4 ++-- .../solr/ltr/norm/TestMinMaxNormalizer.java | 17 ++++++++++++++++- .../solr/ltr/norm/TestStandardNormalizer.java | 17 ++++++++++++++++- 5 files changed, 39 insertions(+), 6 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index fd8a1e8e238..bb134bb7596 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -415,6 +415,9 @@ Bug Fixes * SOLR-11154: Child documents' return fields now include useDocValuesAsStored fields (Mohammed Sheeri Shaketi Nauage via Ishan Chattopadhyaya) +* SOLR-11163: Fix contrib/ltr Normalizer persistence after solr core reload or restart. + (Yuki Yano via Christine Poerschke) + Optimizations ---------------------- diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/norm/MinMaxNormalizer.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/norm/MinMaxNormalizer.java index ff31c0180f8..f6322fefb39 100644 --- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/norm/MinMaxNormalizer.java +++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/norm/MinMaxNormalizer.java @@ -90,8 +90,8 @@ public class MinMaxNormalizer extends Normalizer { @Override public LinkedHashMap paramsToMap() { final LinkedHashMap params = new LinkedHashMap<>(2, 1.0f); - params.put("min", '"'+Float.toString(min)+'"'); - params.put("max", '"'+Float.toString(max)+'"'); + params.put("min", Float.toString(min)); + params.put("max", Float.toString(max)); return params; } diff --git a/solr/contrib/ltr/src/java/org/apache/solr/ltr/norm/StandardNormalizer.java b/solr/contrib/ltr/src/java/org/apache/solr/ltr/norm/StandardNormalizer.java index 57df7b4eb0f..d908ea941ec 100644 --- a/solr/contrib/ltr/src/java/org/apache/solr/ltr/norm/StandardNormalizer.java +++ b/solr/contrib/ltr/src/java/org/apache/solr/ltr/norm/StandardNormalizer.java @@ -82,8 +82,8 @@ public class StandardNormalizer extends Normalizer { @Override public LinkedHashMap paramsToMap() { final LinkedHashMap params = new LinkedHashMap<>(2, 1.0f); - params.put("avg", '"'+Float.toString(avg)+'"'); - params.put("std", '"'+Float.toString(std)+'"'); + params.put("avg", Float.toString(avg)); + params.put("std", Float.toString(std)); return params; } diff --git a/solr/contrib/ltr/src/test/org/apache/solr/ltr/norm/TestMinMaxNormalizer.java b/solr/contrib/ltr/src/test/org/apache/solr/ltr/norm/TestMinMaxNormalizer.java index 794e393a13d..f5e00264d50 100644 --- a/solr/contrib/ltr/src/test/org/apache/solr/ltr/norm/TestMinMaxNormalizer.java +++ b/solr/contrib/ltr/src/test/org/apache/solr/ltr/norm/TestMinMaxNormalizer.java @@ -40,7 +40,7 @@ public class TestMinMaxNormalizer { final MinMaxNormalizer mmn = (MinMaxNormalizer)n; assertEquals(mmn.getMin(), expectedMin, 0.0); assertEquals(mmn.getMax(), expectedMax, 0.0); - assertEquals("{min=\""+expectedMin+"\", max=\""+expectedMax+"\"}", mmn.paramsToMap().toString()); + assertEquals("{min="+expectedMin+", max="+expectedMax+"}", mmn.paramsToMap().toString()); return n; } @@ -118,4 +118,19 @@ public class TestMinMaxNormalizer { value = 5; assertEquals((value - 5f) / (10f - 5f), n.normalize(value), 0.0001); } + + @Test + public void testParamsToMap() { + final MinMaxNormalizer n1 = new MinMaxNormalizer(); + n1.setMin(5.0f); + n1.setMax(10.0f); + + final Map params = n1.paramsToMap(); + final MinMaxNormalizer n2 = (MinMaxNormalizer) Normalizer.getInstance( + new SolrResourceLoader(), + MinMaxNormalizer.class.getName(), + params); + assertEquals(n1.getMin(), n2.getMin(), 1e-6); + assertEquals(n1.getMax(), n2.getMax(), 1e-6); + } } diff --git a/solr/contrib/ltr/src/test/org/apache/solr/ltr/norm/TestStandardNormalizer.java b/solr/contrib/ltr/src/test/org/apache/solr/ltr/norm/TestStandardNormalizer.java index 1794686b1bc..c27dcadd865 100644 --- a/solr/contrib/ltr/src/test/org/apache/solr/ltr/norm/TestStandardNormalizer.java +++ b/solr/contrib/ltr/src/test/org/apache/solr/ltr/norm/TestStandardNormalizer.java @@ -40,7 +40,7 @@ public class TestStandardNormalizer { final StandardNormalizer sn = (StandardNormalizer)n; assertEquals(sn.getAvg(), expectedAvg, 0.0); assertEquals(sn.getStd(), expectedStd, 0.0); - assertEquals("{avg=\""+expectedAvg+"\", std=\""+expectedStd+"\"}", sn.paramsToMap().toString()); + assertEquals("{avg="+expectedAvg+", std="+expectedStd+"}", sn.paramsToMap().toString()); return n; } @@ -130,4 +130,19 @@ public class TestStandardNormalizer { assertEquals((v - 10f) / (1.5f), norm.normalize(v), 0.0001); } } + + @Test + public void testParamsToMap() { + final StandardNormalizer n1 = new StandardNormalizer(); + n1.setAvg(2.0f); + n1.setStd(3.0f); + + final Map params = n1.paramsToMap(); + final StandardNormalizer n2 = (StandardNormalizer) Normalizer.getInstance( + new SolrResourceLoader(), + StandardNormalizer.class.getName(), + params); + assertEquals(n1.getAvg(), n2.getAvg(), 1e-6); + assertEquals(n1.getStd(), n2.getStd(), 1e-6); + } } From c0a6ffe75d11a8ab9c47bcb6a87ac137e07efb6c Mon Sep 17 00:00:00 2001 From: Christine Poerschke Date: Thu, 3 Aug 2017 16:16:44 +0100 Subject: [PATCH 2/2] SOLR-11187: contrib/ltr TestModelManagerPersistence improvements. (Yuki Yano via Christine Poerschke) * in testFeaturePersistence() method fix some assertJDelete vs. assertJQ copy/paste type issues * add testFilePersistence() method --- solr/CHANGES.txt | 2 + .../rest/TestModelManagerPersistence.java | 81 +++++++++++++++++-- 2 files changed, 76 insertions(+), 7 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index bb134bb7596..be89d0632ee 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -125,6 +125,8 @@ Other Changes * SOLR-11140: Remove unused parameter in (private) SolrMetricManager.prepareCloudPlugins method. (Omar Abdelnabi via Christine Poerschke) +* SOLR-11187: contrib/ltr TestModelManagerPersistence improvements. (Yuki Yano via Christine Poerschke) + ================== 7.0.0 ================== Versions of Major Components diff --git a/solr/contrib/ltr/src/test/org/apache/solr/ltr/store/rest/TestModelManagerPersistence.java b/solr/contrib/ltr/src/test/org/apache/solr/ltr/store/rest/TestModelManagerPersistence.java index 9168dd0e95c..9dc28e6fbc2 100644 --- a/solr/contrib/ltr/src/test/org/apache/solr/ltr/store/rest/TestModelManagerPersistence.java +++ b/solr/contrib/ltr/src/test/org/apache/solr/ltr/store/rest/TestModelManagerPersistence.java @@ -23,14 +23,15 @@ import org.apache.commons.io.FileUtils; import org.apache.solr.ltr.TestRerankBase; import org.apache.solr.ltr.feature.ValueFeature; import org.apache.solr.ltr.model.LinearModel; -import org.junit.Before; +import org.apache.solr.ltr.store.FeatureStore; +import org.junit.BeforeClass; import org.junit.Test; import org.noggit.ObjectBuilder; public class TestModelManagerPersistence extends TestRerankBase { - @Before - public void init() throws Exception { + @BeforeClass + public static void init() throws Exception { setupPersistenttest(true); } @@ -98,24 +99,90 @@ public class TestModelManagerPersistence extends TestRerankBase { "/responseHeader/status==0"); assertJQ(ManagedFeatureStore.REST_END_POINT + "/test2", "/features/==[]"); - assertJQ(ManagedModelStore.REST_END_POINT + "/test-model2", + assertJQ(ManagedModelStore.REST_END_POINT, "/models/[0]/name=='test-model'"); restTestHarness.reload(); assertJQ(ManagedFeatureStore.REST_END_POINT + "/test2", "/features/==[]"); - assertJQ(ManagedModelStore.REST_END_POINT + "/test-model2", + assertJQ(ManagedModelStore.REST_END_POINT, "/models/[0]/name=='test-model'"); - assertJDelete(ManagedModelStore.REST_END_POINT + "/test-model1", + assertJDelete(ManagedModelStore.REST_END_POINT + "/test-model", "/responseHeader/status==0"); assertJDelete(ManagedFeatureStore.REST_END_POINT + "/test1", "/responseHeader/status==0"); assertJQ(ManagedFeatureStore.REST_END_POINT + "/test1", "/features/==[]"); + assertJQ(ManagedModelStore.REST_END_POINT, + "/models/==[]"); restTestHarness.reload(); assertJQ(ManagedFeatureStore.REST_END_POINT + "/test1", "/features/==[]"); - + assertJQ(ManagedModelStore.REST_END_POINT, + "/models/==[]"); } + @Test + public void testFilePersistence() throws Exception { + // check whether models and features are empty + assertJQ(ManagedModelStore.REST_END_POINT, + "/models/==[]"); + assertJQ(ManagedFeatureStore.REST_END_POINT + "/" + FeatureStore.DEFAULT_FEATURE_STORE_NAME, + "/features/==[]"); + + // load models and features from files + loadFeatures("features-linear.json"); + loadModels("linear-model.json"); + + // check loaded models and features + final String modelName = "6029760550880411648"; + assertJQ(ManagedModelStore.REST_END_POINT, + "/models/[0]/name=='"+modelName+"'"); + assertJQ(ManagedFeatureStore.REST_END_POINT + "/" + FeatureStore.DEFAULT_FEATURE_STORE_NAME, + "/features/[0]/name=='title'"); + assertJQ(ManagedFeatureStore.REST_END_POINT + "/" + FeatureStore.DEFAULT_FEATURE_STORE_NAME, + "/features/[1]/name=='description'"); + + // check persistence after reload + restTestHarness.reload(); + assertJQ(ManagedModelStore.REST_END_POINT, + "/models/[0]/name=='"+modelName+"'"); + assertJQ(ManagedFeatureStore.REST_END_POINT + "/" + FeatureStore.DEFAULT_FEATURE_STORE_NAME, + "/features/[0]/name=='title'"); + assertJQ(ManagedFeatureStore.REST_END_POINT + "/" + FeatureStore.DEFAULT_FEATURE_STORE_NAME, + "/features/[1]/name=='description'"); + + // check persistence after restart + jetty.stop(); + jetty.start(); + assertJQ(ManagedModelStore.REST_END_POINT, + "/models/[0]/name=='"+modelName+"'"); + assertJQ(ManagedFeatureStore.REST_END_POINT + "/" + FeatureStore.DEFAULT_FEATURE_STORE_NAME, + "/features/[0]/name=='title'"); + assertJQ(ManagedFeatureStore.REST_END_POINT + "/" + FeatureStore.DEFAULT_FEATURE_STORE_NAME, + "/features/[1]/name=='description'"); + + // delete loaded models and features + restTestHarness.delete(ManagedModelStore.REST_END_POINT + "/"+modelName); + restTestHarness.delete(ManagedFeatureStore.REST_END_POINT + "/" + FeatureStore.DEFAULT_FEATURE_STORE_NAME); + assertJQ(ManagedModelStore.REST_END_POINT, + "/models/==[]"); + assertJQ(ManagedFeatureStore.REST_END_POINT + "/" + FeatureStore.DEFAULT_FEATURE_STORE_NAME, + "/features/==[]"); + + // check persistence after reload + restTestHarness.reload(); + assertJQ(ManagedModelStore.REST_END_POINT, + "/models/==[]"); + assertJQ(ManagedFeatureStore.REST_END_POINT + "/" + FeatureStore.DEFAULT_FEATURE_STORE_NAME, + "/features/==[]"); + + // check persistence after restart + jetty.stop(); + jetty.start(); + assertJQ(ManagedModelStore.REST_END_POINT, + "/models/==[]"); + assertJQ(ManagedFeatureStore.REST_END_POINT + "/" + FeatureStore.DEFAULT_FEATURE_STORE_NAME, + "/features/==[]"); + } }