[ 
https://issues.apache.org/jira/browse/OPENNLP-1991?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18126312#comment-18126312
 ] 

ASF GitHub Bot commented on OPENNLP-1991:
-----------------------------------------

krickert commented on code in PR #1323:
URL: https://github.com/apache/opennlp/pull/1323#discussion_r4247904090


##########
opennlp-core/opennlp-formats/src/test/java/opennlp/tools/formats/conllu/ConlluDependencyParserUsageTest.java:
##########
@@ -211,4 +221,49 @@ void testPersistedModelParsesLikeTheOriginal(@TempDir Path 
dir) throws IOExcepti
         reloaded.parse(new String[] {"she", "eats", "fish"},
             new String[] {"PRON", "VERB", "NOUN"}));
   }
+
+  @Test
+  void testFeedforwardTrainSaveReloadAndParse(@TempDir Path dir) throws 
IOException {
+    // The manual's feedforward example: train from the CoNLL-U reader with 
the default
+    // settings, save the model, load it back, and parse with a beam of four. 
The parse
+    // of the reloaded model must equal the parse of the model in memory.
+    final Path treebank = dir.resolve("train.conllu");
+    Files.writeString(treebank, repeat(CONLLU, REPETITIONS), 
StandardCharsets.UTF_8);
+    final Path modelFile = dir.resolve("en-depparse-ff.bin");
+    final InputStreamFactory trainingData = () -> 
Files.newInputStream(treebank);
+
+    FeedforwardDependencyModel model;
+    try (ObjectStream<DependencySample> samples =
+        new ConlluDependencySampleStream(trainingData, ConlluTagset.U)) {
+      model = FeedforwardDependencyTrainer.train(
+          samples, FeedforwardDependencyTrainer.Settings.defaults());
+    }
+    try (OutputStream out = Files.newOutputStream(modelFile)) {
+      model.serialize(out);
+    }
+
+    DependencyParser reloaded = new FeedforwardDependencyParser(
+        FeedforwardDependencyModel.load(modelFile), BEAM_SIZE);
+    final DependencyTree expected =
+        new FeedforwardDependencyParser(model, BEAM_SIZE).parse(ALICE_TOKENS, 
ALICE_TAGS);
+    final DependencyTree parse = reloaded.parse(ALICE_TOKENS, ALICE_TAGS);
+    assertEquals(expected, parse);
+    assertEquals(ALICE_TOKENS.length, parse.size());
+    assertEquals(ALICE_TREE.root(), parse.root());
+  }
+
+  /**
+   * Repeats CoNLL-U content so a small fixture gives the trainers enough 
evidence.
+   *
+   * @param conllu The content to repeat; sentences are separated by a blank 
line.
+   * @param times How often to repeat it.
+   * @return The repeated content. Never {@code null}.
+   */
+  private static String repeat(String conllu, int times) {

Review Comment:
   Uses `String.repeat`, the local is `feedforwardModel`, and the test asserts 
`ALICE_TREE` after the save and reload.



##########
opennlp-eval-tests/src/test/java/opennlp/tools/eval/UniversalDependencyParserEval.java:
##########
@@ -50,9 +53,12 @@
  * and both again over the tokens that are not tagged {@code PUNCT}, the 
customary
  * reporting convention for Universal Dependencies.</p>
  *
- * <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}

Review Comment:
   Now `{@link DependencyParserME}` and "the English EWT and Spanish AnCora 
development splits".



##########
opennlp-docs/src/docbkx/dependency.xml:
##########
@@ -91,6 +93,32 @@ opennlp DependencyParserME en-depparse.bin < 
tagged-sentences.txt]]>
                </para>
        </section>
 
+       <section xml:id="tools.dependency.feedforward">
+               <title>Feedforward parser</title>
+               <para>
+                       The feedforward trainer has its own versioned model 
format. This example trains,
+                       saves, reloads, and creates a parser with a beam size 
of four:
+                       <programlisting language="java">
+<![CDATA[FeedforwardDependencyModel model;
+try (ObjectStream<DependencySample> samples =
+    new ConlluDependencySampleStream(trainingData, ConlluTagset.U)) {
+  model = FeedforwardDependencyTrainer.train(
+      samples, FeedforwardDependencyTrainer.Settings.defaults());
+}
+try (OutputStream out = Files.newOutputStream(Path.of("en-depparse-ff.bin"))) {
+  model.serialize(out);
+}
+
+DependencyParser parser = new FeedforwardDependencyParser(
+    FeedforwardDependencyModel.load(Path.of("en-depparse-ff.bin")), 4);]]>
+                       </programlisting>
+               </para>
+               <para>

Review Comment:
   The section is down to the example (now `serialize(Path)` and the `Path` 
constructor, like the section above), one sentence on greedy versus beam and 
thread sharing, and the evaluation paragraph names which parser is scored on 
which split.





> Add a feedforward neural dependency parser
> ------------------------------------------
>
>                 Key: OPENNLP-1991
>                 URL: https://issues.apache.org/jira/browse/OPENNLP-1991
>             Project: OpenNLP
>          Issue Type: Improvement
>            Reporter: Kristian Rickert
>            Assignee: Kristian Rickert
>            Priority: Major
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> Add a feedforward neural implementation of the dependency parser API 
> introduced by OPENNLP-547. Use OpenNLP’s standard model container, artifact 
> serializers, TrainingParameters, factory, and CLI conventions. Include 
> deterministic training, greedy and beam decoding, model round-trip tests, and 
> held-out Universal Dependencies evaluation. Address the deferred review 
> feedback on shared scoring/training logic and parser-owned caches.  Implement 
> this without new dependencies.
> The list above may still get this ticket to further split.  For now this is a 
> placeholder to capture the code that was moved out of OPENNLP-547.  I'll edit 
> after 547 is merged.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to