OPENNLP-1991: Add a feedforward neural dependency parser by krickert · Pull Request #1323 · apache/opennlp

krickert · 2026-09-20T18:21:29Z

Adds FeedforwardDependencyParser, an arc-standard neural dependency parser after Chen and Manning (2014). It implements the same DependencyParser API as DependencyParserME and decodes greedily or with a beam.

FeedforwardDependencyTrainer trains the model from TrainingParameters and can refine it with beam search and early updates. FeedforwardDependencyModel extends BaseModel and stores the network as an artifact with its own serializer. The scoring cache is parser-owned, opt-in and bounded in bytes. StringUtil.toLowerCase now applies the Unicode final sigma rule, which the parser uses to key its word vocabulary.

The manual, the CoNLL-U usage test and UniversalDependencyParserEval cover the new parser. The evaluation has greedy and beam 4 cases for English EWT and Spanish AnCora; the pinned scores will be refreshed from an eval build on this branch before the PR leaves draft.

Remaining before 3.0.0: factory and CLI integration, which will be a follow-up PR.

krickert · 2026-10-01T22:48:19Z

After 547 merges, this is a small ticket that piggy backs on it. Just needs a rebase when it merges.

mawiesne · 2026-10-08T20:02:05Z

@krickert Please go for rebase and harmonization of this branch, pls. Just want to make sure, things are consistent for ppl to inspect the PR draft against main.

krickert · 2026-10-08T20:19:27Z

On it

krickert · 2026-10-08T21:16:42Z

https://ci-builds.apache.org/job/OpenNLP/job/eval-tests-configurable/97/console

Harmonizing

rzo1

Hi @krickert - as usual here is the first round of an AI review with our usual review bar:

The findings are inline (5 blocking, 32 minor). Process points:

The PR is marked ready, but the remaining work listed in its description (model/factory/CLI integration, shared scoring, parser-owned caches, deterministic vocabulary, fresh evaluations) changes public API and the model format, both of which freeze at 3.0.0. Please move it back to draft until that has landed.
The pinned feedforward scores (UniversalDependencyParserEval.java:246/259) depend on the vocabulary order and cover greedy decoding only, while the manual uses beam 4. After the ordering fix we need an eval build to re-pin them, with a beam row added.
Squash message: please drop the "Rebased onto main after OPENNLP-547…" paragraph and the Co-authored-by/Reviewed-by trailers, replace the Assisted-by/Co-authored-by: Claude lines with at most one ASF-conformant Generated-by: trailer, and keep the body to 2-3 sentences.

rzo1 · 2026-10-09T07:55:57Z

+ * @since 3.0.0
+ */
+@ThreadSafe
+public class FeedforwardDependencyModel {


Blocking: A public *Model that does not extend BaseModel adds a second model contract (no manifest, language, version check or serialVersionUID, own MAGIC format), while the sibling DependencyModel extends BaseModel. This freezes at 3.0.0. Please extend BaseModel and store the weights as an artifact entry with its own serializer, or keep the type package-private until the model/factory integration lands.

rzo1 · 2026-10-09T07:55:57Z

+   *                   not be negative.
+   * @param seed The random seed making a run reproducible.
+   */
+  public record Settings(int embeddingSize, int hiddenSize, int epochs, int batchSize,


Blocking: The public Settings record (nine components, seed included) and the static train/refine methods add a hyperparameter API that bypasses TrainingParameters. Existing pattern: DependencyParserME.train(languageCode, samples, TrainingParameters). Please accept TrainingParameters and keep Settings package-private as the parsed form, or keep the trainer non-public until the standard training integration is done.

rzo1 · 2026-10-09T07:55:57Z

+   * same values. Training and refinement use uncached copies; {@link #copy()}
+   * does not copy the cache.</p>
+   */
+  synchronized void enableScoringCache() {


Blocking: The scoring cache is bounded by entry count, not bytes, and every parser constructor enables it, also for models loaded from untrusted files (FeedforwardDependencyParser.java:82). With CACHE_ENTRY_LIMIT = 32_768 entries of double[hiddenSize] and MAX_HIDDEN_SIZE = 65_536, a ~10 MB model can grow the cache to ~17 GB. Line 252 also allocates an AtomicReferenceArray(rows) per position (36) up front. Please budget the cache in bytes, size each position's table to the vocabulary it reads, and make the cache parser-owned and opt-in.

Also: the Javadoc gives rationale, not behaviour, e.g. "Enables the scoring cache; subsequent calls have no effect."

rzo1 · 2026-10-09T07:55:57Z

+        Arrays.fill(outputBiasGradient, 0.0);
+        embeddingGradients.clear();
+
+        for (int b = batchStart; b < batchEnd; b++) {


Blocking: The network math exists in several copies: optimize() inlines a full forward/backward pass that GlobalOptimizer.forward/backward (555/595) already implement apart from dropout; FeedforwardDependencyModel.score is a third forward pass; log-softmax is written four times although ArrayMath.logSumOfExps exists; FeedforwardDependencyScoringTest:254-275 tests two copies by reflection. Please use one package-private forward/backward helper with an optional dropout mask and one log-softmax, tested directly. Until then the class Javadoc at FeedforwardDependencyModel.java:49 ("the same class scores configurations during training and decoding") is incorrect.

rzo1 · 2026-10-09T07:55:57Z

+   * @throws IllegalArgumentException Thrown if a parameter is {@code null} or no
+   *         trainable example can be derived from the samples.
+   */
+  public static FeedforwardDependencyModel train(ObjectStream<DependencySample> samples,


Blocking: @throws is incomplete on train and refine: missing IllegalStateException from checkFinite after a diverging epoch (1049/1129, 329); for train also the IllegalArgumentException for exceeding MAX_MODEL_FLOAT_VALUES (791) and for non-finite pretrained values (712).

rzo1 · 2026-10-09T07:55:58Z

+    parser = new FeedforwardDependencyParser(model);
+  }
+
+  @Test


Minor: The eight tests lack Javadoc (the comment at 102-103 belongs there) and repeat the DependencyParserEdgeCaseTest contract (empty input, null token/tag, single token, round trip, thread sharing, same THREADS/ITERATIONS_PER_THREAD); the thread test covers greedy only. Please run the shared contract as a @ParameterizedTest over both parsers and include beam size 4.

rzo1 · 2026-10-09T07:55:58Z

+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/** Checks numerical precision and concurrent cache use during dependency parsing. */
+class FeedforwardDependencyScoringTest {


Minor: Package-private while siblings are public; constants 52-57 lack Javadoc; repeats via an IntStream.range(0, 8) round argument instead of @RepeatedTest; reads private fields (cache, remaining, byPosition) by reflection. Please test through package-private accessors once the cache is parser-owned.

rzo1 · 2026-10-09T07:55:58Z

+   * @param times How often to repeat it.
+   * @return The repeated content. Never {@code null}.
+   */
+  private static String repeat(String conllu, int times) {


Minor: repeat re-implements String.repeat: (CONLLU + "\n\n").repeat(REPETITIONS). The local model (235) shadows the static DependencyModel model (106), and the test checks only self-consistency and the root, not ALICE_TREE. Please rename the local and assert the gold tree, or say why the toy model cannot reach it.

rzo1 · 2026-10-09T07:55:58Z

- * <p>The parser is trained with a feature cutoff of {@value #FEATURE_CUTOFF} and
- * evaluated on English, German, Spanish, and French. The English cross validation uses
- * {@value #ENGLISH_FOLDS} folds of the training split.</p>
+ * <p>The classical parser is trained with a feature cutoff of {@value #FEATURE_CUTOFF}


Minor: "the classical parser" is informal, please use {@link DependencyParserME}. Lines 60-61 read as if the English treebank were AnCora: "English EWT and Spanish AnCora development splits".

rzo1 · 2026-10-09T07:55:58Z

+    FeedforwardDependencyModel.load(Path.of("en-depparse-ff.bin")), 4);]]>
+			</programlisting>
+		</para>
+		<para>


Minor: This paragraph documents an internal failure mode. The example (108) uses serialize(OutputStream) unlike DependencyModel.serialize(Path) in the section above, and the evaluation paragraph (153-160) still describes a single parser. Please keep one or two sentences on greedy vs beam decoding, Settings.defaults() and thread safety, and state which parser is evaluated on which split.

krickert · 2026-10-11T09:12:09Z

https://ci-builds.apache.org/job/OpenNLP/job/eval-tests-configurable/100/console

Lucky 100 runnig

krickert · 2026-10-11T15:10:51Z

Eval failed - small patch to fix it. On it now

krickert · 2026-10-11T16:36:10Z

Patch applied, eval build 101 is running. Will flip to ready for review once that is done.

Adds FeedforwardDependencyParser, an arc-standard neural parser after Chen and Manning (2014) that implements the same DependencyParser API as DependencyParserME, with greedy and beam decoding. FeedforwardDependencyTrainer trains the FeedforwardDependencyModel from TrainingParameters and refines it with beam search and early updates; the model is a BaseModel with the network as an artifact. StringUtil.toLowerCase applies the Unicode final sigma rule, and the manual, the CoNLL-U usage test and the UD evaluation cover the new parser. Generated-by: Claude Code Signed-off-by: Kristian Rickert <kristian@apache.org> Co-authored-by: Claude Code 2.1.296 (Claude Opus 5.5)

krickert · 2026-10-12T01:36:35Z

https://ci-builds.apache.org/job/OpenNLP/job/eval-tests-configurable/102/

101 revealed a change that was needed. Re-running locally (should be about 2x faster) while 102 runs.

This was referenced Sep 24, 2026

Review round 1 for apache#1236: dependency parser fixes ai-pipestream/opennlp#8

Merged

Review round 1 for apache#1323: carry the #1236 review over ai-pipestream/opennlp#10

Merged

rzo1 mentioned this pull request Sep 25, 2026

OPENNLP-547: Add a transition-based dependency parser component #1236

Merged

krickert mentioned this pull request Sep 26, 2026

Review round 2 for apache#1236: rzo1's 2026-09-25 list ai-pipestream/opennlp#23

Merged

krickert changed the base branch from OPENNLP-547-dependency-parser to main September 29, 2026 05:18

krickert changed the title ~~OPENNLP-1991: Add a feedforward neural dependency parser~~ OPENNLP-1991 (part of OPENNLP-547): Add a feedforward neural dependency parser Oct 1, 2026

This was referenced Oct 2, 2026

OPENNLP-1935: Regex removal (10/10): Forbid java.util.regex in production code #1348

Merged

OPENNLP-1991: Use ArgumentChecks in the feedforward parser (for #1323) ai-pipestream/opennlp#32

Draft

rzo1 added new feature New functionality (Jira: New Feature) component: parser Parser and dependency parser labels Oct 4, 2026

rzo1 assigned krickert Oct 4, 2026

mawiesne changed the title ~~OPENNLP-1991 (part of OPENNLP-547): Add a feedforward neural dependency parser~~ OPENNLP-1991: Add a feedforward neural dependency parser Oct 8, 2026

mawiesne added this to the 3.1.0 milestone Oct 8, 2026

krickert force-pushed the OPENNLP-1991-feedforward-dependency-parser branch from 6e889ec to 1519c75 Compare October 8, 2026 21:13

krickert marked this pull request as ready for review October 8, 2026 21:16

rzo1 reviewed Oct 9, 2026

View reviewed changes

mawiesne modified the milestones: 3.1.0, 3.0.0 Oct 9, 2026

krickert marked this pull request as draft October 11, 2026 08:14

krickert force-pushed the OPENNLP-1991-feedforward-dependency-parser branch from 1519c75 to c1e5fa1 Compare October 11, 2026 08:17

krickert mentioned this pull request Oct 11, 2026

Add command-line tools for the feedforward dependency parser #1469

Open

krickert mentioned this pull request Oct 11, 2026

GH-1469: Add command-line tools for the feedforward dependency parser #1473

Draft

krickert force-pushed the OPENNLP-1991-feedforward-dependency-parser branch from c1e5fa1 to f0d8072 Compare October 11, 2026 15:14

rzo1 mentioned this pull request Oct 11, 2026

GH-1476: Let the parsers take a caller-supplied POSTagger and Chunker #1478

Draft

krickert force-pushed the OPENNLP-1991-feedforward-dependency-parser branch from f0d8072 to ff09c3f Compare October 11, 2026 22:30

Conversation

krickert commented Sep 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

krickert commented Oct 1, 2026

Uh oh!

mawiesne commented Oct 8, 2026

Uh oh!

krickert commented Oct 8, 2026

Uh oh!

krickert commented Oct 8, 2026

Uh oh!

rzo1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krickert commented Oct 11, 2026

Uh oh!

krickert commented Oct 11, 2026

Uh oh!

krickert commented Oct 11, 2026

Uh oh!

krickert commented Oct 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

krickert commented Sep 20, 2026 •

edited

Loading