You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opennlp.apache.org by kr...@apache.org on 2026/08/22 01:01:09 UTC
(opennlp) 15/15: relation: Address review: enforce layer contract, unbox chain walk
This is an automated email from the ASF dual-hosted git repository.
krickert pushed a commit to branch OPENNLP-1920-relation-extraction
in repository https://gitbox.apache.org/repos/asf/opennlp.git
commit 66f74f474d47bc38c3fc47e316e77636b71631ac
Author: Kristian Rickert <kr...@gmail.com>
AuthorDate: Sat Aug 8 19:08:24 2026 -0400
relation: Address review: enforce layer contract, unbox chain walk
Validate required layers through DocumentAnnotators.requireLayers so an
absent layer is rejected with the shared message and empty layers
degrade to an empty relations layer, matching every other annotator.
Walk root chains over int arrays instead of boxed lists, fold the
repeated dependency layer fixture into one test helper, and cover
triggers with leading whitespace.
---
.../opennlp/tools/relation/RelationAnnotator.java | 68 ++++++++++++----------
.../tools/relation/RelationAnnotatorTest.java | 35 ++++++-----
.../tools/relation/RelationPatternTest.java | 9 ++-
3 files changed, 64 insertions(+), 48 deletions(-)
diff --git a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/relation/RelationAnnotator.java b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/relation/RelationAnnotator.java
index b7923f32d..8317044da 100644
--- a/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/relation/RelationAnnotator.java
+++ b/opennlp-core/opennlp-runtime/src/main/java/opennlp/tools/relation/RelationAnnotator.java
@@ -27,6 +27,7 @@ import opennlp.tools.depparse.DependencyArc;
import opennlp.tools.document.Annotation;
import opennlp.tools.document.Document;
import opennlp.tools.document.DocumentAnnotator;
+import opennlp.tools.document.DocumentAnnotators;
import opennlp.tools.document.LayerKey;
import opennlp.tools.document.Layers;
import opennlp.tools.util.Span;
@@ -93,29 +94,29 @@ public class RelationAnnotator implements DocumentAnnotator {
* entities share a head token, or whose heads are not connected in the dependency
* graph, contributes no relation.</p>
*
- * @param document The document to annotate. Must not be {@code null} and must carry a
- * non-empty {@link Layers#TOKENS} layer and a
- * {@link DependencyAnnotator#DEPENDENCIES} layer holding exactly one
- * arc per token. An absent {@link Layers#ENTITIES} layer reads as
- * empty, which yields an empty {@link #RELATIONS} layer.
+ * @param document The document to annotate. Must not be {@code null} and must carry
+ * the {@link Layers#TOKENS}, {@link Layers#ENTITIES}, and
+ * {@link DependencyAnnotator#DEPENDENCIES} layers, the dependency
+ * layer holding exactly one arc per token. The layers may be empty: a
+ * document without tokens or without entities yields an empty
+ * {@link #RELATIONS} layer.
* @return A new {@link Document} with the {@link #RELATIONS} layer added. Never
* {@code null}.
- * @throws IllegalArgumentException Thrown if {@code document} is {@code null}, the
- * token layer is absent or empty, the dependency layer does not hold exactly
- * one arc per token, two arcs share a dependent, an arc refers to a token
+ * @throws IllegalArgumentException Thrown if {@code document} is {@code null}, a
+ * required layer is absent, the dependency layer does not hold exactly one
+ * arc per token, two arcs share a dependent, an arc refers to a token
* outside the token layer, or the document already carries the
* {@link #RELATIONS} layer.
*/
@Override
public Document annotate(Document document) {
- if (document == null) {
- throw new IllegalArgumentException("document must not be null");
- }
+ DocumentAnnotators.requireLayers(document, Layers.TOKENS, Layers.ENTITIES,
+ DependencyAnnotator.DEPENDENCIES);
final List<Annotation<String>> tokens = document.get(Layers.TOKENS);
final List<Annotation<String>> entities = document.get(Layers.ENTITIES);
final List<Annotation<DependencyArc>> arcs =
document.get(DependencyAnnotator.DEPENDENCIES);
- if (tokens.isEmpty() || arcs.size() != tokens.size()) {
+ if (arcs.size() != tokens.size()) {
throw new IllegalArgumentException("document needs aligned " + Layers.TOKENS
+ " and " + DependencyAnnotator.DEPENDENCIES + " layers");
}
@@ -136,21 +137,21 @@ public class RelationAnnotator implements DocumentAnnotator {
// Each entity's chain to the root depends only on that entity, so walking it once
// per entity keeps the pair loop below from repeating the walk for every partner.
final int[] entityHeads = new int[entities.size()];
- final List<List<Integer>> chains = new ArrayList<>(entities.size());
+ final int[][] chains = new int[entities.size()][];
for (int e = 0; e < entities.size(); e++) {
entityHeads[e] = entityHead(entities.get(e).span(), tokens, heads);
- chains.add(entityHeads[e] < 0 ? null : chainToRoot(entityHeads[e], heads));
+ chains[e] = entityHeads[e] < 0 ? null : chainToRoot(entityHeads[e], heads);
}
final List<Annotation<RelationMention>> mentions = new ArrayList<>();
for (int subject = 0; subject < entities.size(); subject++) {
for (int object = 0; object < entities.size(); object++) {
if (subject == object || entityHeads[subject] == entityHeads[object]
- || chains.get(subject) == null || chains.get(object) == null) {
+ || chains[subject] == null || chains[object] == null) {
continue;
}
matchPair(tokens, relations, entities, subject, object,
- chains.get(subject), chains.get(object), mentions);
+ chains[subject], chains[object], mentions);
}
}
return document.with(RELATIONS, mentions);
@@ -184,15 +185,17 @@ public class RelationAnnotator implements DocumentAnnotator {
private void matchPair(List<Annotation<String>> tokens,
String[] relations, List<Annotation<String>> entities,
int subject, int object,
- List<Integer> subjectChain, List<Integer> objectChain,
+ int[] subjectChain, int[] objectChain,
List<Annotation<RelationMention>> mentions) {
int pivotOnSubject = -1;
int pivotOnObject = -1;
- for (int o = 0; o < objectChain.size() && pivotOnSubject < 0; o++) {
- final int index = subjectChain.indexOf(objectChain.get(o));
- if (index >= 0) {
- pivotOnSubject = index;
- pivotOnObject = o;
+ for (int o = 0; o < objectChain.length && pivotOnSubject < 0; o++) {
+ for (int s = 0; s < subjectChain.length; s++) {
+ if (subjectChain[s] == objectChain[o]) {
+ pivotOnSubject = s;
+ pivotOnObject = o;
+ break;
+ }
}
}
if (pivotOnSubject < 0) {
@@ -201,12 +204,12 @@ public class RelationAnnotator implements DocumentAnnotator {
final List<String> steps = new ArrayList<>();
for (int s = 0; s < pivotOnSubject; s++) {
- steps.add(RelationPattern.UP_STEP + relations[subjectChain.get(s)]);
+ steps.add(RelationPattern.UP_STEP + relations[subjectChain[s]]);
}
for (int o = pivotOnObject - 1; o >= 0; o--) {
- steps.add(RelationPattern.DOWN_STEP + relations[objectChain.get(o)]);
+ steps.add(RelationPattern.DOWN_STEP + relations[objectChain[o]]);
}
- final int pivot = subjectChain.get(pivotOnSubject);
+ final int pivot = subjectChain[pivotOnSubject];
final String pivotForm = StringUtil.toLowerCase(tokens.get(pivot).value());
for (int p = 0; p < patterns.size(); p++) {
@@ -268,14 +271,17 @@ public class RelationAnnotator implements DocumentAnnotator {
* {@code null} when the walk takes more steps than there are tokens, which
* only happens when the arcs contain a cycle.
*/
- private List<Integer> chainToRoot(int start, int[] heads) {
- final List<Integer> chain = new ArrayList<>();
- int current = start;
- while (current != DependencyArc.ROOT_HEAD) {
- if (chain.size() > heads.length) {
+ private int[] chainToRoot(int start, int[] heads) {
+ int length = 0;
+ for (int current = start; current != DependencyArc.ROOT_HEAD; current = heads[current]) {
+ if (++length > heads.length) {
return null;
}
- chain.add(current);
+ }
+ final int[] chain = new int[length];
+ int current = start;
+ for (int i = 0; i < length; i++) {
+ chain[i] = current;
current = heads[current];
}
return chain;
diff --git a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/relation/RelationAnnotatorTest.java b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/relation/RelationAnnotatorTest.java
index 259d375fc..f872277b4 100644
--- a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/relation/RelationAnnotatorTest.java
+++ b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/relation/RelationAnnotatorTest.java
@@ -47,6 +47,23 @@ public class RelationAnnotatorTest {
/** A single no-break space, U+00A0, whitespace to the toolkit but not to the JDK. */
private static final String NO_BREAK_SPACE = "\u00A0";
+ /**
+ * Anchors each dependency arc on its dependent token's span, the layout
+ * {@link DependencyAnnotator} produces.
+ *
+ * @param tokens The token layer the arcs index into.
+ * @param arcs The arcs to annotate.
+ * @return One annotation per arc, in arc order. Never {@code null}.
+ */
+ private static List<Annotation<DependencyArc>> dependencyLayer(
+ List<Annotation<String>> tokens, List<DependencyArc> arcs) {
+ final List<Annotation<DependencyArc>> dependencies = new ArrayList<>(arcs.size());
+ for (final DependencyArc arc : arcs) {
+ dependencies.add(new Annotation<>(tokens.get(arc.dependent()).span(), arc));
+ }
+ return dependencies;
+ }
+
/**
* Builds the parsed document "Acme Corp acquired Bolt in 2024." with entities for
* Acme Corp (index 0), Bolt (index 1), and 2024 (index 2). The dependency layer marks
@@ -74,17 +91,13 @@ public class RelationAnnotatorTest {
new DependencyArc(5, 4, "case"),
new DependencyArc(2, 5, "obl"),
new DependencyArc(2, 6, "punct"));
- final List<Annotation<DependencyArc>> dependencies = new ArrayList<>();
- for (final DependencyArc arc : arcs) {
- dependencies.add(new Annotation<>(tokens.get(arc.dependent()).span(), arc));
- }
return Document.of(text)
.with(Layers.TOKENS, tokens)
.with(Layers.ENTITIES, List.of(
new Annotation<>(new Span(0, 9), "organization"),
new Annotation<>(new Span(19, 23), "organization"),
new Annotation<>(new Span(27, 31), "date")))
- .with(DependencyAnnotator.DEPENDENCIES, dependencies);
+ .with(DependencyAnnotator.DEPENDENCIES, dependencyLayer(tokens, arcs));
}
/**
@@ -292,13 +305,9 @@ public class RelationAnnotatorTest {
new DependencyArc(1, 0, "nsubj"),
new DependencyArc(DependencyArc.ROOT_HEAD, 1, "root"),
new DependencyArc(1, 2, "punct"));
- final List<Annotation<DependencyArc>> dependencies = new ArrayList<>();
- for (final DependencyArc arc : arcs) {
- dependencies.add(new Annotation<>(tokens.get(arc.dependent()).span(), arc));
- }
return Document.of(text)
.with(Layers.TOKENS, tokens)
- .with(DependencyAnnotator.DEPENDENCIES, dependencies);
+ .with(DependencyAnnotator.DEPENDENCIES, dependencyLayer(tokens, arcs));
}
/**
@@ -413,16 +422,12 @@ public class RelationAnnotatorTest {
new DependencyArc(4, 3, "case"),
new DependencyArc(2, 4, "nmod"),
new DependencyArc(0, 5, "punct"));
- final List<Annotation<DependencyArc>> dependencies = new ArrayList<>();
- for (final DependencyArc arc : arcs) {
- dependencies.add(new Annotation<>(tokens.get(arc.dependent()).span(), arc));
- }
return Document.of(text)
.with(Layers.TOKENS, tokens)
.with(Layers.ENTITIES, List.of(
new Annotation<>(new Span(0, 8), "location"),
new Annotation<>(new Span(18, 22), "organization")))
- .with(DependencyAnnotator.DEPENDENCIES, dependencies);
+ .with(DependencyAnnotator.DEPENDENCIES, dependencyLayer(tokens, arcs));
}
/**
diff --git a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/relation/RelationPatternTest.java b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/relation/RelationPatternTest.java
index 8aad61d82..78b212e12 100644
--- a/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/relation/RelationPatternTest.java
+++ b/opennlp-core/opennlp-runtime/src/test/java/opennlp/tools/relation/RelationPatternTest.java
@@ -226,8 +226,9 @@ public class RelationPatternTest {
* Verifies that a trigger containing whitespace is rejected at construction: the
* trigger is compared against a single pivot token, which the tokenizer guarantees
* carries no whitespace, so a multi-word or padded trigger is a rule that could
- * never match. The whitespace scan follows the project predicate, so a no-break
- * space inside or around the trigger is caught the same way as a plain space.
+ * never match. The scan covers whitespace inside, before, and after the trigger and
+ * follows the project predicate, so a no-break space is caught the same way as a
+ * plain space.
*/
@Test
void testTriggerContainingWhitespaceIsRejected() {
@@ -237,8 +238,12 @@ public class RelationPatternTest {
Assertions.assertEquals("trigger must not contain whitespace, since it is matched"
+ " against a single token: new york", spaced.getMessage());
+ Assertions.assertThrows(IllegalArgumentException.class,
+ () -> new RelationPattern("t", "<nsubj", " founded"));
Assertions.assertThrows(IllegalArgumentException.class,
() -> new RelationPattern("t", "<nsubj", "founded" + NBSP));
+ Assertions.assertThrows(IllegalArgumentException.class,
+ () -> new RelationPattern("t", "<nsubj", NBSP + "founded"));
Assertions.assertThrows(IllegalArgumentException.class,
() -> new RelationPattern("t", "<nsubj", String.valueOf(NBSP)));
}