#3822 - Order of matches found in knowledge base search is not correct

- Properly additional match labels in the SPARQLQueryBuilder - Use the additional match labels when ranking using the LevenshteinFeatureGenerator
inception-project · Feb 21, 2023 · 03cc990 · 03cc990
1 parent b72c8e1
commit 03cc990
Show file tree

Hide file tree

Showing 11 changed files with 144 additions and 22 deletions.
diff --git a/...java/de/tudarmstadt/ukp/inception/conceptlinking/feature/LevenshteinFeatureGenerator.java b/...java/de/tudarmstadt/ukp/inception/conceptlinking/feature/LevenshteinFeatureGenerator.java
@@ -24,7 +24,9 @@
 import static de.tudarmstadt.ukp.inception.conceptlinking.model.CandidateEntity.KEY_LEVENSHTEIN_QUERY_NC;
 import static de.tudarmstadt.ukp.inception.conceptlinking.model.CandidateEntity.KEY_MENTION;
 import static de.tudarmstadt.ukp.inception.conceptlinking.model.CandidateEntity.KEY_MENTION_CONTEXT;
+import static de.tudarmstadt.ukp.inception.conceptlinking.model.CandidateEntity.KEY_MENTION_NC;
 import static de.tudarmstadt.ukp.inception.conceptlinking.model.CandidateEntity.KEY_QUERY;
+import static de.tudarmstadt.ukp.inception.conceptlinking.model.CandidateEntity.KEY_QUERY_NC;
 import static org.apache.commons.lang3.StringUtils.join;
 
 import org.apache.commons.text.similarity.LevenshteinDistance;
@@ -47,26 +49,32 @@ public class LevenshteinFeatureGenerator
     public void apply(CandidateEntity aCandidate)
     {
         String label = aCandidate.getLabel();
-        String labelNC = aCandidate.getLabel().toLowerCase(aCandidate.getLocale());
+        update(aCandidate, label);
+        aCandidate.getHandle().getMatchTerms().forEach(p -> update(aCandidate, p.getKey()));
+    }
 
-        aCandidate.get(KEY_MENTION) //
-                .map(mention -> lev.apply(label, mention)) //
-                .ifPresent(score -> aCandidate.put(KEY_LEVENSHTEIN_MENTION, score));
+    private void update(CandidateEntity aCandidate, String label)
+    {
+        String labelNC = label.toLowerCase(aCandidate.getLocale());
 
-        aCandidate.get(KEY_MENTION) //
+        aCandidate.get(KEY_MENTION_NC) //
                 .map(mention -> lev.apply(labelNC, mention)) //
-                .ifPresent(score -> aCandidate.put(KEY_LEVENSHTEIN_MENTION_NC, score));
+                .ifPresent(score -> aCandidate.mergeMin(KEY_LEVENSHTEIN_MENTION_NC, score));
 
-        aCandidate.get(KEY_QUERY) //
-                .map(query -> lev.apply(label, query)) //
-                .ifPresent(score -> aCandidate.put(KEY_LEVENSHTEIN_QUERY, score));
+        aCandidate.get(KEY_QUERY_NC) //
+                .map(query -> lev.apply(labelNC, query)) //
+                .ifPresent(score -> aCandidate.mergeMin(KEY_LEVENSHTEIN_QUERY_NC, score));
+
+        aCandidate.get(KEY_MENTION) //
+                .map(mention -> lev.apply(label, mention)) //
+                .ifPresent(score -> aCandidate.mergeMin(KEY_LEVENSHTEIN_MENTION, score));
 
         aCandidate.get(KEY_QUERY) //
-                .map(query -> lev.apply(labelNC, query)) //
-                .ifPresent(score -> aCandidate.put(KEY_LEVENSHTEIN_QUERY_NC, score));
+                .map(query -> lev.apply(label, query)) //
+                .ifPresent(score -> aCandidate.mergeMin(KEY_LEVENSHTEIN_QUERY, score));
 
         aCandidate.get(KEY_MENTION_CONTEXT) //
                 .map(context -> lev.apply(label, join(context, ' '))) //
-                .ifPresent(score -> aCandidate.put(KEY_LEVENSHTEIN_MENTION_CONTEXT, score));
+                .ifPresent(score -> aCandidate.mergeMin(KEY_LEVENSHTEIN_MENTION_CONTEXT, score));
     }
 }
diff --git a/...king/src/main/java/de/tudarmstadt/ukp/inception/conceptlinking/model/CandidateEntity.java b/...king/src/main/java/de/tudarmstadt/ukp/inception/conceptlinking/model/CandidateEntity.java
@@ -220,6 +220,12 @@ public <T> T put(Key<T> aKey, T aValue)
         }
     }
 
+    public int mergeMin(Key<Integer> aKey, int aValue)
+    {
+        return (int) features.merge(aKey.name, aValue,
+                (o, n) -> o == null ? n : Math.min((int) o, (int) n));
+    }
+
     public Map<String, Object> getFeatures()
     {
         return unmodifiableMap(features);

diff --git a/...n/java/de/tudarmstadt/ukp/inception/conceptlinking/service/ConceptLinkingServiceImpl.java b/...n/java/de/tudarmstadt/ukp/inception/conceptlinking/service/ConceptLinkingServiceImpl.java
@@ -294,7 +294,7 @@ private void findStartingWithMatches(Set<KBHandle> result, KnowledgeBase aKB,
         }
 
         var duration = currentTimeMillis() - startTime;
-        log.debug("Found [{}] candidates starting with [{}]] in {}ms", startingWithMatches.size(),
+        log.debug("Found [{}] candidates starting with [{}] in {}ms", startingWithMatches.size(),
                 aQuery, duration);
         WicketUtil.serverTiming("findStartingWithMatches", duration);
 
@@ -399,7 +399,7 @@ private CandidateEntity initCandidate(CandidateEntity candidate, String aQuery,
 
         candidate.put(KEY_LABEL_NC, candidate.getLabel().toLowerCase(candidate.getLocale()));
 
-        if (aCas != null) {
+        if (aCas != null && aMention != null) {
             AnnotationFS sentence = selectSentenceCovering(aCas, aBegin);
             if (sentence != null) {
                 List<String> mentionContext = new ArrayList<>();
@@ -423,6 +423,7 @@ private CandidateEntity initCandidate(CandidateEntity candidate, String aQuery,
                 log.warn("Mention sentence could not be determined. Skipping.");
             }
         }
+
         return candidate;
     }
 

diff --git a/inception/inception-kb/src/main/java/de/tudarmstadt/ukp/inception/kb/graph/KBHandle.java b/inception/inception-kb/src/main/java/de/tudarmstadt/ukp/inception/kb/graph/KBHandle.java
@@ -17,13 +17,16 @@
  */
 package de.tudarmstadt.ukp.inception.kb.graph;
 
+import static java.util.Collections.emptySet;
 import static org.apache.commons.lang3.builder.ToStringStyle.SHORT_PREFIX_STYLE;
 
 import java.util.ArrayList;
 import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Set;
 
 import org.apache.commons.lang3.builder.ToStringBuilder;
 import org.apache.commons.lang3.tuple.Pair;
@@ -36,7 +39,7 @@ public class KBHandle
     private static final long serialVersionUID = -4284462837460396185L;
     private String identifier;
     private String name;
-    private List<Pair<String, String>> matchTerms;
+    private Set<Pair<String, String>> matchTerms;
     private String description;
     private KnowledgeBase kb;
     private String language;
@@ -157,14 +160,18 @@ public void setName(String aName)
     public void addMatchTerm(String aLabel, String aLanguage)
     {
         if (matchTerms == null) {
-            matchTerms = new ArrayList<>();
+            matchTerms = new LinkedHashSet<>();
         }
 
         matchTerms.add(Pair.of(aLabel, aLanguage));
     }
 
-    public List<Pair<String, String>> getMatchTerms()
+    public Set<Pair<String, String>> getMatchTerms()
     {
+        if (matchTerms == null) {
+            return emptySet();
+        }
+
         return matchTerms;
     }
 

diff --git a/...ion-kb/src/main/java/de/tudarmstadt/ukp/inception/kb/querybuilder/SPARQLQueryBuilder.java b/...ion-kb/src/main/java/de/tudarmstadt/ukp/inception/kb/querybuilder/SPARQLQueryBuilder.java
@@ -2123,16 +2123,27 @@ private List<KBHandle> reduceRedundantResults(List<KBHandle> aHandles)
             // Not recorded yet -> add it
             if (current == null) {
                 cMap.put(handle.getIdentifier(), handle);
+                continue;
             }
+
+            boolean replace = false;
             // Found one with a label while current one doesn't have one
-            else if (current.getName() == null && handle.getName() != null) {
-                cMap.put(handle.getIdentifier(), handle);
+            if (current.getName() == null && handle.getName() != null) {
+                replace = true;
             }
             // Found an exact language match -> use that one instead
             // Note that having a language implies that there is a label!
             else if (kb.getDefaultLanguage() != null
                     && kb.getDefaultLanguage().equals(handle.getLanguage())) {
+                replace = true;
+            }
+
+            if (replace) {
                 cMap.put(handle.getIdentifier(), handle);
+                current.getMatchTerms().forEach(e -> handle.addMatchTerm(e.getKey(), e.getValue()));
+            }
+            else {
+                handle.getMatchTerms().forEach(e -> current.addMatchTerm(e.getKey(), e.getValue()));
             }
         }
 

diff --git a/...n-kb/src/test/java/de/tudarmstadt/ukp/inception/kb/querybuilder/FusekiRepositoryTest.java b/...n-kb/src/test/java/de/tudarmstadt/ukp/inception/kb/querybuilder/FusekiRepositoryTest.java
@@ -101,11 +101,13 @@ public void tearDown()
 
     private static List<Arguments> tests() throws Exception
     {
-        // These require additional configuration in Fuseki FTS
         var exclusions = asList( //
+                // These require additional configuration in Fuseki FTS
                 "thatMatchingAgainstAdditionalSearchPropertiesWorks", //
                 "testWithLabelMatchingExactlyAnyOf_subproperty", //
-                "testWithLabelStartingWith_OLIA");
+                "testWithLabelStartingWith_OLIA",
+                // This test returns one match term less than in the RDF4J case - not clear why
+                "thatMatchingAgainstAdditionalSearchPropertiesWorks2");
 
         return SPARQLQueryBuilderTest.tests().stream() //
                 .filter(scenario -> !exclusions.contains(scenario.name))

diff --git a/...on-kb/src/test/java/de/tudarmstadt/ukp/inception/kb/querybuilder/Rdf4JRepositoryTest.java b/...on-kb/src/test/java/de/tudarmstadt/ukp/inception/kb/querybuilder/Rdf4JRepositoryTest.java
@@ -20,19 +20,29 @@
 import static de.tudarmstadt.ukp.inception.kb.IriConstants.FTS_LUCENE;
 import static de.tudarmstadt.ukp.inception.kb.http.PerThreadSslCheckingHttpClientUtils.restoreSslVerification;
 import static de.tudarmstadt.ukp.inception.kb.http.PerThreadSslCheckingHttpClientUtils.suspendSslVerification;
+import static de.tudarmstadt.ukp.inception.kb.querybuilder.SPARQLQueryBuilderTest.DATA_ADDITIONAL_SEARCH_PROPERTIES_2;
+import static de.tudarmstadt.ukp.inception.kb.querybuilder.SPARQLQueryBuilderTest.TURTLE_PREFIX;
+import static de.tudarmstadt.ukp.inception.kb.querybuilder.SPARQLQueryBuilderTest.importDataFromString;
 import static java.util.Arrays.asList;
+import static org.assertj.core.api.Assertions.contentOf;
+import static org.eclipse.rdf4j.rio.RDFFormat.TURTLE;
 
 import java.lang.reflect.Method;
 import java.util.List;
 import java.util.stream.Collectors;
 
+import org.apache.wicket.util.file.File;
+import org.eclipse.rdf4j.query.BindingSet;
+import org.eclipse.rdf4j.query.TupleQueryResult;
 import org.eclipse.rdf4j.repository.Repository;
 import org.eclipse.rdf4j.repository.RepositoryConnection;
 import org.eclipse.rdf4j.repository.sail.SailRepository;
 import org.eclipse.rdf4j.sail.lucene.LuceneSail;
 import org.eclipse.rdf4j.sail.memory.MemoryStore;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestInfo;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.Arguments;
@@ -102,4 +112,23 @@ public void runTests(String aScenarioName, Scenario aScenario) throws Exception
     {
         aScenario.implementation.accept(repository, kb);
     }
+
+    @Disabled("Not actually a test but rather a playground for SPARQL queries")
+    @Test
+    void runSparqlQuery() throws Exception
+    {
+        try (RepositoryConnection conn = repository.getConnection()) {
+            importDataFromString(repository, kb, TURTLE, TURTLE_PREFIX,
+                    DATA_ADDITIONAL_SEARCH_PROPERTIES_2);
+
+            var tupleQuery = conn.prepareTupleQuery(contentOf(new File(
+                    "src/test/resources/queries/additional_search_properties_2/rdf4j.sparql")));
+            try (TupleQueryResult result = tupleQuery.evaluate()) {
+                while (result.hasNext()) {
+                    BindingSet bindings = result.next();
+                    System.out.println(bindings);
+                }
+            }
+        }
+    }
 }
diff --git a/...kb/src/test/java/de/tudarmstadt/ukp/inception/kb/querybuilder/SPARQLQueryBuilderTest.java b/...kb/src/test/java/de/tudarmstadt/ukp/inception/kb/querybuilder/SPARQLQueryBuilderTest.java
@@ -47,6 +47,7 @@
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.function.FailableBiConsumer;
+import org.apache.commons.lang3.tuple.Pair;
 import org.eclipse.rdf4j.model.impl.SimpleValueFactory;
 import org.eclipse.rdf4j.model.vocabulary.OWL;
 import org.eclipse.rdf4j.model.vocabulary.RDF;
@@ -100,6 +101,9 @@ public class SPARQLQueryBuilderTest
     static final String DATA_ADDITIONAL_SEARCH_PROPERTIES = contentOf(
             new File("src/test/resources/turtle/data_additional_search_properties.ttl"), UTF_8);
 
+    static final String DATA_ADDITIONAL_SEARCH_PROPERTIES_2 = contentOf(
+            new File("src/test/resources/turtle/data_additional_search_properties_2.ttl"), UTF_8);
+
     static final String LABEL_SUBPROPERTY = String.join("\n", //
             "<#sublabel>", //
             "    rdfs:subPropertyOf rdfs:label .", //
@@ -273,6 +277,8 @@ static List<Scenario> tests() throws Exception
                         SPARQLQueryBuilderTest::thatSearchOverMultipleLabelsWorks),
                 new Scenario("thatMatchingAgainstAdditionalSearchPropertiesWorks",
                         SPARQLQueryBuilderTest::thatMatchingAgainstAdditionalSearchPropertiesWorks),
+                new Scenario("thatMatchingAgainstAdditionalSearchPropertiesWorks2",
+                        SPARQLQueryBuilderTest::thatMatchingAgainstAdditionalSearchPropertiesWorks2),
                 new Scenario("thatExistsReturnsFalseWhenDataQueriedForDoesNotExist",
                         SPARQLQueryBuilderTest::thatExistsReturnsFalseWhenDataQueriedForDoesNotExist),
                 new Scenario("thatExplicitClassCanBeRetrievedByItsIdentifier",
@@ -478,6 +484,41 @@ static void thatMatchingAgainstAdditionalSearchPropertiesWorks(Repository aRepos
         }
     }
 
+    static void thatMatchingAgainstAdditionalSearchPropertiesWorks2(Repository aRepository,
+            KnowledgeBase aKB)
+        throws Exception
+    {
+        aKB.setLabelIri("http://www.w3.org/2000/01/rdf-schema#prefLabel");
+        aKB.setAdditionalMatchingProperties(asList("http://www.w3.org/2000/01/rdf-schema#label"));
+
+        importDataFromString(aRepository, aKB, TURTLE, TURTLE_PREFIX,
+                DATA_ADDITIONAL_SEARCH_PROPERTIES_2);
+
+        var queriesWithMatchTerms = asList(//
+                Pair.of("hand", //
+                        asList("Hand structure (body structure)", "Hand structure", "Hand")),
+                Pair.of("hand structure", //
+                        asList("Hand structure (body structure)", "Hand structure", "Hand")),
+                Pair.of("body structure", //
+                        asList("Hand structure (body structure)", "Hand structure")));
+
+        for (var queryPair : queriesWithMatchTerms) {
+            List<KBHandle> results = asHandles(aRepository, SPARQLQueryBuilder //
+                    .forItems(aKB) //
+                    .withLabelMatchingAnyOf(queryPair.getKey()) //
+                    .retrieveLabel());
+
+            var expectedKBHandle = new KBHandle("http://example.org/#example",
+                    "Hand structure (body structure)");
+            queryPair.getValue().forEach(v -> expectedKBHandle.addMatchTerm(v, null));
+
+            assertThat(results) //
+                    .usingRecursiveFieldByFieldElementComparatorOnFields("identifier", "name",
+                            "matchTerms") //
+                    .containsExactlyInAnyOrder(expectedKBHandle);
+        }
+    }
+
     /**
      * Checks that {@code SPARQLQueryBuilder#exists(RepositoryConnection, boolean)} can return
      * {@code false} by querying for the parent of a root class in

diff --git a/...ption/inception-kb/src/test/resources/queries/additional_search_properties_2/rdf4j.sparql b/...ption/inception-kb/src/test/resources/queries/additional_search_properties_2/rdf4j.sparql
@@ -0,0 +1,13 @@
+PREFIX search: <http://www.openrdf.org/contrib/lucenesail#>
+          SELECT DISTINCT ?m ?l ?subj
+          WHERE { { { { ?pMatch <http://www.w3.org/2000/01/rdf-schema#subPropertyOf> * <http://www.w3.org/2000/01/rdf-schema#prefLabel> . } UNION { ?pMatch <http://www.w3.org/2000/01/rdf-schema#subPropertyOf> * <http://www.w3.org/2000/01/rdf-schema#label> . }
+          { ?subj search:matches [ search:query "hand" ;
+              search:property ?pMatch ;
+              search:snippet ?snippet ] .
+          BIND( REPLACE( REPLACE( ?snippet, "</B>", "" ), "<B>", "" ) AS ?label )
+          ?subj ?pMatch ?m .
+          FILTER ( ( STR( ?label ) = STR( ?m ) && ( LANGMATCHES( LANG( ?m ), "en" ) || LANG( ?m ) = "" ) ) ) } } }
+          OPTIONAL { ?pPrefLabel <http://www.w3.org/2000/01/rdf-schema#subPropertyOf> * <http://www.w3.org/2000/01/rdf-schema#prefLabel> . }
+          OPTIONAL { { ?subj ?pPrefLabel ?l .
+          FILTER ( ( LANGMATCHES( LANG( ?l ), "en" ) || LANG( ?l ) = "" ) ) } } }
+          LIMIT 200
diff --git a/inception/inception-kb/src/test/resources/turtle/data_additional_search_properties.ttl b/inception/inception-kb/src/test/resources/turtle/data_additional_search_properties.ttl
@@ -2,4 +2,4 @@
     rdfs:prefLabel 'specimen' ;
     rdfs:label 'sample' ;
     rdfs:label 'instance' ;
-    rdfs:label 'case'  .
+    rdfs:label 'case' .
diff --git a/inception/inception-kb/src/test/resources/turtle/data_additional_search_properties_2.ttl b/inception/inception-kb/src/test/resources/turtle/data_additional_search_properties_2.ttl
@@ -0,0 +1,4 @@
+<#example>
+    rdfs:prefLabel 'Hand structure (body structure)' ;
+    rdfs:label 'Hand' ;
+    rdfs:label 'Hand structure' .