diff options
author | MariusArhaug <mariusarhaug@hotmail.com> | 2024-04-24 15:04:35 +0200 |
---|---|---|
committer | MariusArhaug <mariusarhaug@hotmail.com> | 2024-04-30 10:29:52 +0200 |
commit | 491888936e16df91d93f37a5504efcfbfbfc95db (patch) | |
tree | 3d8e326de64618779929f8f86a49f190248d0fde | |
parent | aaae1863d50586fb86685d3cbb6c73f589d3370c (diff) |
Fix CR comments
10 files changed, 55 insertions, 60 deletions
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/component/SignificanceModelRegistry.java b/config-model/src/main/java/com/yahoo/vespa/model/container/component/SignificanceModelRegistry.java index ce9dc2bce0f..693eebd75a8 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/component/SignificanceModelRegistry.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/component/SignificanceModelRegistry.java @@ -32,8 +32,9 @@ public class SignificanceModelRegistry extends SimpleComponent implements Signif super(new ComponentModel(BundleInstantiationSpecification.fromStrings(CLASS, CLASS, BUNDLE))); if (spec != null) { - for (Element modelElement : XML.getChildren(spec, "model")) { - addConfig(Model.fromXml(deployState, modelElement, Set.of(SIGNIFICANCE_MODEL)).modelReference()); + for (Element modelElement : XML.getChildren(spec, "model")) { + addConfig(Model.fromXml(deployState, modelElement, Set.of(SIGNIFICANCE_MODEL)).modelReference()); + } } } @@ -56,7 +57,7 @@ public class SignificanceModelRegistry extends SimpleComponent implements Signif } - class SignificanceModelConfig { + static class SignificanceModelConfig { private final ModelReference path; public SignificanceModelConfig(ModelReference path) { diff --git a/container-search/src/test/java/com/yahoo/search/significance/model/en.json b/container-search/src/test/java/com/yahoo/search/significance/model/en.json index 50bae5e3451..04010959a58 100644 --- a/container-search/src/test/java/com/yahoo/search/significance/model/en.json +++ b/container-search/src/test/java/com/yahoo/search/significance/model/en.json @@ -2,13 +2,17 @@ "version" : "1.0", "id" : "test::1", "description" : "desc", - "corpus-size" : 10, - "language" : "en", - "word-count" : 4, - "frequencies" : { - "usa" : 2, - "hello": 3, - "world": 5, - "test": 2 + "languages" : { + "en": { + "description" : "english model", + "document-count" : 10, + "language" : "en", + "document-frequencies" : { + "usa" : 2, + "hello": 3, + "world": 5, + "test": 2 + } + } } } diff --git a/container-search/src/test/java/com/yahoo/search/significance/test/SignificanceSearcherTest.java b/container-search/src/test/java/com/yahoo/search/significance/test/SignificanceSearcherTest.java index 890db3abb51..ad39c49bc1b 100644 --- a/container-search/src/test/java/com/yahoo/search/significance/test/SignificanceSearcherTest.java +++ b/container-search/src/test/java/com/yahoo/search/significance/test/SignificanceSearcherTest.java @@ -15,7 +15,9 @@ import com.yahoo.search.significance.SignificanceSearcher; import org.junit.jupiter.api.Test; import java.nio.file.Path; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import static com.yahoo.test.JunitCompat.assertEquals; @@ -30,10 +32,10 @@ public class SignificanceSearcherTest { SignificanceSearcher searcher; public SignificanceSearcherTest() { - HashMap<Language, Path> map = new HashMap<>(); - map.put(Language.ENGLISH, Path.of("src/test/java/com/yahoo/search/significance/model/en.json")); + List<Path> models = new ArrayList<>(); + models.add( Path.of("src/test/java/com/yahoo/search/significance/model/en.json")); - significanceModelRegistry = new DefaultSignificanceModelRegistry(map); + significanceModelRegistry = new DefaultSignificanceModelRegistry(models); searcher = new SignificanceSearcher(significanceModelRegistry); } diff --git a/linguistics/src/main/java/com/yahoo/language/significance/impl/DefaultSignificanceModelRegistry.java b/linguistics/src/main/java/com/yahoo/language/significance/impl/DefaultSignificanceModelRegistry.java index 7bf05c71342..72874c15d9e 100644 --- a/linguistics/src/main/java/com/yahoo/language/significance/impl/DefaultSignificanceModelRegistry.java +++ b/linguistics/src/main/java/com/yahoo/language/significance/impl/DefaultSignificanceModelRegistry.java @@ -2,7 +2,6 @@ package com.yahoo.language.significance.impl; import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.SerializationConfig; import com.yahoo.component.annotation.Inject; import com.yahoo.language.Language; import com.yahoo.language.significance.SignificanceModel; @@ -10,10 +9,12 @@ import com.yahoo.language.significance.SignificanceModelRegistry; import com.yahoo.search.significance.config.SignificanceConfig; import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.file.Path; import java.util.EnumMap; import java.util.List; import java.util.Map; +import java.util.Optional; /** * Default implementation of {@link SignificanceModelRegistry}. @@ -28,42 +29,32 @@ public class DefaultSignificanceModelRegistry implements SignificanceModelRegist @Inject public DefaultSignificanceModelRegistry(SignificanceConfig cfg) { this.models = new EnumMap<>(Language.class); - ObjectMapper objectMapper = new ObjectMapper(); for (var model : cfg.model()) { - try { - SignificanceModelFile file = objectMapper.readValue(model.path().toFile(), SignificanceModelFile.class); - - for (var pair : file.languages().entrySet()) { - addModel( - Language.fromLanguageTag(pair.getKey()), new DefaultSignificanceModel(pair.getValue(), file.id())); - } - } catch (IOException e) { - throw new RuntimeException("Failed to load model from " + model.path(), e); - } + addModel(model.path()); } } public DefaultSignificanceModelRegistry(List<Path> models) { this.models = new EnumMap<>(Language.class); + for (var path : models) { + addModel(path); + } + } + public void addModel(Path path) { ObjectMapper objectMapper = new ObjectMapper(); - for (var model : models) { - try { - SignificanceModelFile file = objectMapper.readValue(model.toFile(), SignificanceModelFile.class); - for (var pair : file.languages().entrySet()) { - addModel( - Language.fromLanguageTag(pair.getKey()), new DefaultSignificanceModel(pair.getValue(), file.id())); - } - } catch (IOException e) { - throw new RuntimeException("Failed to load model from " + model, e); + try { + SignificanceModelFile file = objectMapper.readValue(path.toFile(), SignificanceModelFile.class); + for (var pair : file.languages().entrySet()) { + this.models.put( + Language.fromLanguageTag(pair.getKey()), + new DefaultSignificanceModel(pair.getValue(), file.id())); } + } catch (IOException e) { + throw new UncheckedIOException("Failed to load model from " + path, e); } } - public void addModel(Language lang, SignificanceModel model) { - this.models.put(lang, model); - } - @Override public Optional<SignificanceModel> getModel(Language language) { if (!models.containsKey(language)) diff --git a/linguistics/src/main/java/com/yahoo/language/significance/impl/DocumentFrequencyFile.java b/linguistics/src/main/java/com/yahoo/language/significance/impl/DocumentFrequencyFile.java index c2ad1de8fd8..b62754ac8ad 100644 --- a/linguistics/src/main/java/com/yahoo/language/significance/impl/DocumentFrequencyFile.java +++ b/linguistics/src/main/java/com/yahoo/language/significance/impl/DocumentFrequencyFile.java @@ -1,3 +1,4 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.language.significance.impl; import com.fasterxml.jackson.annotation.JsonCreator; @@ -7,16 +8,17 @@ import com.fasterxml.jackson.annotation.JsonProperty; import java.util.HashMap; +/** + * + * @author MariusArhaug + */ @JsonIgnoreProperties(ignoreUnknown = true) @JsonInclude(JsonInclude.Include.NON_NULL) public class DocumentFrequencyFile { private final String description; - private final String language; - private final int documentCount; - private final int wordCount; private final HashMap<String, Long> frequencies; @@ -24,13 +26,9 @@ public class DocumentFrequencyFile { public DocumentFrequencyFile( @JsonProperty("description") String description, @JsonProperty("document-count") int documentCount, - @JsonProperty("word-count") int wordCount, - @JsonProperty("language") String language, @JsonProperty("document-frequencies") HashMap<String, Long> frequencies) { this.description = description; this.documentCount = documentCount; - this.wordCount = wordCount; - this.language = language; this.frequencies = frequencies; } @@ -40,12 +38,6 @@ public class DocumentFrequencyFile { @JsonProperty("document-count") public int documentCount() { return documentCount; } - @JsonProperty("language") - public String language() { return language; } - - @JsonProperty("word-count") - public int wordCount() { return wordCount; } - @JsonProperty("document-frequencies") public HashMap<String, Long> frequencies() { return frequencies; } } diff --git a/linguistics/src/main/java/com/yahoo/language/significance/impl/SignificanceModelFile.java b/linguistics/src/main/java/com/yahoo/language/significance/impl/SignificanceModelFile.java index cb923fe8861..902613379f0 100644 --- a/linguistics/src/main/java/com/yahoo/language/significance/impl/SignificanceModelFile.java +++ b/linguistics/src/main/java/com/yahoo/language/significance/impl/SignificanceModelFile.java @@ -1,3 +1,4 @@ +// Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.language.significance.impl; import com.fasterxml.jackson.annotation.JsonCreator; @@ -8,6 +9,10 @@ import com.fasterxml.jackson.annotation.JsonProperty; import java.util.HashMap; import java.util.List; +/** + * + * @author MariusArhaug + */ @JsonIgnoreProperties(ignoreUnknown = true) public class SignificanceModelFile { diff --git a/linguistics/src/test/java/com/yahoo/language/significance/DefaultSignificanceModelRegistryTest.java b/linguistics/src/test/java/com/yahoo/language/significance/DefaultSignificanceModelRegistryTest.java index 833caca3b24..e8594885b9e 100644 --- a/linguistics/src/test/java/com/yahoo/language/significance/DefaultSignificanceModelRegistryTest.java +++ b/linguistics/src/test/java/com/yahoo/language/significance/DefaultSignificanceModelRegistryTest.java @@ -64,8 +64,14 @@ public class DefaultSignificanceModelRegistryTest { DefaultSignificanceModelRegistry defaultSignificanceModelRegistry = new DefaultSignificanceModelRegistry(models); - var englishModel = defaultSignificanceModelRegistry.getModel(Language.ENGLISH); - var norwegianModel = defaultSignificanceModelRegistry.getModel(Language.NORWEGIAN_BOKMAL); + var optionalEnglishModel = defaultSignificanceModelRegistry.getModel(Language.ENGLISH); + var optionalNorwegianModel = defaultSignificanceModelRegistry.getModel(Language.NORWEGIAN_BOKMAL); + + assertTrue(optionalEnglishModel.isPresent()); + assertTrue(optionalNorwegianModel.isPresent()); + + var englishModel = optionalEnglishModel.get(); + var norwegianModel = optionalNorwegianModel.get(); assertNotNull(englishModel); assertNotNull(norwegianModel); diff --git a/linguistics/src/test/models/docv1.json b/linguistics/src/test/models/docv1.json index dd211840ede..04010959a58 100644 --- a/linguistics/src/test/models/docv1.json +++ b/linguistics/src/test/models/docv1.json @@ -7,7 +7,6 @@ "description" : "english model", "document-count" : 10, "language" : "en", - "word-count" : 4, "document-frequencies" : { "usa" : 2, "hello": 3, diff --git a/linguistics/src/test/models/docv2.json b/linguistics/src/test/models/docv2.json index c7f621b9acb..c00d02fb744 100644 --- a/linguistics/src/test/models/docv2.json +++ b/linguistics/src/test/models/docv2.json @@ -6,8 +6,6 @@ "en": { "description" : "english model", "document-count" : 14, - "language" : "en", - "word-count" : 5, "document-frequencies" : { "usa" : 2, "hello": 3, @@ -19,8 +17,6 @@ "nb": { "description" : "norwegian model", "document-count" : 20, - "language" : "en", - "word-count" : 7, "document-frequencies" : { "usa" : 2, "hello": 10, diff --git a/linguistics/src/test/models/en.json b/linguistics/src/test/models/en.json index a46b6a5b51f..87b7b2faa08 100644 --- a/linguistics/src/test/models/en.json +++ b/linguistics/src/test/models/en.json @@ -12,4 +12,3 @@ "test": 2 } } -}
\ No newline at end of file |