Skip to content

Commit

Permalink
Merge branch 'release/35.x'
Browse files Browse the repository at this point in the history 
* release/35.x:
  #5274 - Link merged to wrong host when candidate hosts only differ in links
  • Loading branch information
@reckart
reckart committed Feb 3, 2025
2 parents 4dd1981 + 48fe99b commit 2dd25b0
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 335 deletions.
39 changes: 25 additions & 14 deletions ...ration/src/main/java/de/tudarmstadt/ukp/inception/curation/merge/CasMergeLinkFeature.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,15 @@ private static Annotation findLinkSourceInTargetCas(CAS aTargetCas, AnnotationFS
{
var candidateHosts = selectBestCandidateSlotHost(aTargetCas, adapter, aSourceFs,
slotFeature, sourceLink);

if (candidateHosts.isEmpty()) {
throw new UnfulfilledPrerequisitesException(
"There is no suitable [" + adapter.getLayer().getUiName() + "] annotation at ["
+ aSourceFs.getBegin() + "," + aSourceFs.getEnd()
+ "] into which the link could be merged. Please add one first.");
}
var targetFS = candidateHosts.get();
return targetFS;

return candidateHosts.get();
}

private static AnnotationFS findLinkTargetInTargetCas(CasMergeContext aContext,
Expand Down Expand Up @@ -164,9 +165,8 @@ private static LinkWithRoleModel existingLinkWithTarget(LinkWithRoleModel aLink,
return null;
}

private static Optional<Annotation> selectBestCandidateSlotHost(CAS aTargetCas,
TypeAdapter aAdapter, AnnotationFS aSourceFS, AnnotationFeature aSlotFeature,
LinkWithRoleModel aSourceLink)
static Optional<Annotation> selectBestCandidateSlotHost(CAS aTargetCas, TypeAdapter aAdapter,
AnnotationFS aSourceFS, AnnotationFeature aSlotFeature, LinkWithRoleModel aSourceLink)
{
var targetType = aAdapter.getAnnotationType(aTargetCas);
if (targetType.isEmpty()) {
Expand Down Expand Up @@ -207,10 +207,7 @@ private static Optional<Annotation> selectBestCandidateSlotHost(CAS aTargetCas,
}

// Still more than one, then we need to look at the slots...
List<LinkWithRoleModel> sourceLinks = aAdapter.getFeatureValue(aSlotFeature, aSourceFS);
var matSourceLinks = sourceLinks.stream() //
.map(link -> toMaterializedLink(aSourceFS, aSlotFeature, link)) //
.toList();
var matSourceLinks = getMaterializedLinks(aAdapter, aSourceFS);

var filterestCandidateTargets2 = filteredCandidateTargets.stream() //
.sorted(comparing(candidateTarget -> countLinkDifference(aAdapter, aSourceFS,
Expand All @@ -224,11 +221,7 @@ private static int countLinkDifference(TypeAdapter aAdapter, AnnotationFS aSourc
AnnotationFeature aSlotFeature, LinkWithRoleModel aSourceLink,
List<MaterializedLink> matSourceLinks, Annotation candidateTarget)
{
List<LinkWithRoleModel> targetLinks = aAdapter.getFeatureValue(aSlotFeature,
candidateTarget);
var matTargetLinks = targetLinks.stream() //
.map(link -> toMaterializedLink(candidateTarget, aSlotFeature, link)) //
.collect(toCollection(ArrayList::new));
var matTargetLinks = getMaterializedLinks(aAdapter, candidateTarget);

// Let's assume we would already have merged the link - this may in the best case either
// reduce the difference to 0 or increase it to 1 if we added it once to often
Expand All @@ -245,4 +238,22 @@ private static int countLinkDifference(TypeAdapter aAdapter, AnnotationFS aSourc

return unmatchedSourceLinks.size() + matTargetLinks.size();
}

private static ArrayList<MaterializedLink> getMaterializedLinks(TypeAdapter aAdapter,
AnnotationFS candidateTarget)
{
var matTargetLinks = new ArrayList<MaterializedLink>();
var linkFeatures = aAdapter.listFeatures().stream() //
.filter(f -> f.getLinkMode() != LinkMode.NONE) //
.toList();
for (var linkFeature : linkFeatures) {
List<LinkWithRoleModel> targetLinks = aAdapter.getFeatureValue(linkFeature,
candidateTarget);
targetLinks.stream() //
.map(link -> toMaterializedLink(candidateTarget, linkFeature, link)) //
.forEach(matTargetLinks::add);
}

return matTargetLinks;
}
}
39 changes: 39 additions & 0 deletions ...-curation/src/test/java/de/tudarmstadt/ukp/inception/curation/merge/CasMergeLinkTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static de.tudarmstadt.ukp.clarin.webanno.model.OverlapMode.ANY_OVERLAP;
import static de.tudarmstadt.ukp.inception.annotation.feature.link.LinkFeatureDiffMode.INCLUDE;
import static de.tudarmstadt.ukp.inception.annotation.feature.link.LinkFeatureMultiplicityMode.ONE_TARGET_MULTIPLE_ROLES;
import static de.tudarmstadt.ukp.inception.curation.merge.CurationTestUtils.ALT_LINKS_FEATURE;
import static de.tudarmstadt.ukp.inception.curation.merge.CurationTestUtils.HOST_TYPE;
import static de.tudarmstadt.ukp.inception.curation.merge.CurationTestUtils.LINKS_FEATURE;
import static de.tudarmstadt.ukp.inception.curation.merge.CurationTestUtils.TARGET_FEATURE;
Expand Down Expand Up @@ -279,6 +280,44 @@ public void thatSecondLinkWithSameTargetAndSameRoleIsRejectedWhenRolesAreEnabled
new LinkWithRoleModel("role1", null, targetFiller.getAddress()));
}

@Test
public void thatLinkIsAttachedToCorrectStackedTargetWhenOtherLinkFeatureDiffers()
throws Exception
{
// Set up source CAS
var sourceFs = buildAnnotation(sourceCas1.getCas(), HOST_TYPE) //
.at(0, 0) //
.withFeature(LINKS_FEATURE, asList(makeLinkFS(sourceCas1, "role1", 1, 1)))
.withFeature(ALT_LINKS_FEATURE, asList(makeLinkFS(sourceCas1, "role1", 2, 2)))
.buildAndAddToIndexes();

// Set up target CAS
new NamedEntity(targetCas, 2, 2).addToIndexes();
var targetCandidateFs1 = buildAnnotation(targetCas.getCas(), HOST_TYPE) //
.at(sourceFs) //
.withFeature(LINKS_FEATURE, asList(makeLinkFS(targetCas, "role1", 3, 3))) //
.buildAndAddToIndexes();
var targetCandidateFs2 = buildAnnotation(targetCas.getCas(), HOST_TYPE) //
.at(sourceFs) //
.withFeature(LINKS_FEATURE, asList(makeLinkFS(targetCas, "role1", 1, 1))) //
.buildAndAddToIndexes();

// Perform merge
sut.mergeSlotFeature(document, DUMMY_USER, slotLayer, targetCas.getCas(), sourceFs,
ALT_LINKS_FEATURE, 0);

var adapter = schemaService.getAdapter(slotLayer);
var altLinksFeature = adapter.getFeature(ALT_LINKS_FEATURE).get();
List<LinkWithRoleModel> mergedLinks1 = adapter.getFeatureValue(altLinksFeature,
targetCandidateFs1);
assertThat(mergedLinks1) //
.as("Link has NOT been merged to the first candidate").isEmpty();
List<LinkWithRoleModel> mergedLinks2 = adapter.getFeatureValue(altLinksFeature,
targetCandidateFs2);
assertThat(mergedLinks2) //
.as("Link has been merged to the second candidate").isNotEmpty();
}

@Nested
class SingleUserThesholdBasedMergeStrategyTests
{
Expand Down
20 changes: 19 additions & 1 deletion ...-curation/src/test/java/de/tudarmstadt/ukp/inception/curation/merge/CasMergeTestBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ public class CasMergeTestBase
protected AnnotationFeature depFlavorFeature;
protected AnnotationLayer slotLayer;
protected AnnotationFeature slotFeature;
protected AnnotationFeature slotFeature2;
protected AnnotationFeature stringFeature;
protected AnnotationLayer multiValRel;
protected AnnotationFeature multiValRelRel1;
Expand All @@ -116,6 +117,8 @@ public void setup() throws Exception
slotHostDiffAdapter = new SpanDiffAdapter(HOST_TYPE);
slotHostDiffAdapter.addLinkFeature("links", "role", "target", ONE_TARGET_MULTIPLE_ROLES,
EXCLUDE);
slotHostDiffAdapter.addLinkFeature("altLinks", "role", "target", ONE_TARGET_MULTIPLE_ROLES,
EXCLUDE);

diffAdapters = new ArrayList<>();
// diffAdapters.add(TOKEN_DIFF_ADAPTER);
Expand Down Expand Up @@ -240,6 +243,21 @@ public void setup() throws Exception
slotFeature.setVisible(true);
slotFeature.setCuratable(true);

slotFeature2 = new AnnotationFeature();
slotFeature2.setName("altLinks");
slotFeature2.setEnabled(true);
slotFeature2.setType(Token.class.getName());
slotFeature2.setMode(MultiValueMode.ARRAY);
slotFeature2.setLinkMode(LinkMode.WITH_ROLE);
slotFeature2.setLinkTypeName(CurationTestUtils.LINK_TYPE);
slotFeature2.setLinkTypeRoleFeatureName("role");
slotFeature2.setLinkTypeTargetFeatureName("target");
slotFeature2.setUiName("links");
slotFeature2.setLayer(slotLayer);
slotFeature2.setProject(project);
slotFeature2.setVisible(true);
slotFeature2.setCuratable(true);

stringFeature = new AnnotationFeature();
stringFeature.setName("f1");
stringFeature.setEnabled(true);
Expand Down Expand Up @@ -350,7 +368,7 @@ public void setup() throws Exception
return asList(neFeature, neIdentifierFeature);
}
if (type.getName().equals(HOST_TYPE)) {
return asList(slotFeature, stringFeature);
return asList(slotFeature, slotFeature2, stringFeature);
}
if (type.getName().equals("webanno.custom.Multivalrel")) {
return asList(multiValRelRel1, multiValRelRel2);
Expand Down
7 changes: 4 additions & 3 deletions ...curation/src/test/java/de/tudarmstadt/ukp/inception/curation/merge/CurationTestUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public class CurationTestUtils
public static final String TARGET_FEATURE = "target";
public static final String ROLE_FEATURE = "role";
public static final String LINKS_FEATURE = "links";
public static final String ALT_LINKS_FEATURE = "altLinks";
public static final String HOST_TYPE = "webanno.custom.LinkHost";
public static final String LINK_TYPE = "webanno.custom.LinkType";

Expand Down Expand Up @@ -271,12 +272,12 @@ public static AnnotationFS makeLinkHostFS(JCas aCas, int aBegin, int aEnd, Featu
public static FeatureStructure makeLinkFS(JCas aCas, String aRole, int aTargetBegin,
int aTargetEnd)
{
var token = new NamedEntity(aCas, aTargetBegin, aTargetEnd);
token.addToIndexes();
var filler = new NamedEntity(aCas, aTargetBegin, aTargetEnd);
filler.addToIndexes();

return buildFS(aCas.getCas(), LINK_TYPE) //
.withFeature(ROLE_FEATURE, aRole) //
.withFeature(TARGET_FEATURE, token) //
.withFeature(TARGET_FEATURE, filler) //
.buildAndAddToIndexes();
}
}
Loading

0 comments on commit 2dd25b0

Please sign in to comment.