Repository navigation
Add bounded batch parsing to HermitCrab - #582
johnml1135 wants to merge 4 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #582 +/- ##
==========================================
+ Coverage 74.93% 75.08% +0.14%
==========================================
Files 454 456 +2
Lines 38490 38569 +79
Branches 5322 5331 +9
==========================================
+ Hits 28843 28958 +115
+ Misses 8495 8463 -32
+ Partials 1152 1148 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
Require degree-1 compilation for batch parsing, preserve ordered per-word results and errors, and serialize traced batches. Keep alternative budgets on invocation-local rule wrappers and retain the existing constructor default.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
d47ef0a to
9485394
Compare
This comment has been minimized.
This comment has been minimized.
|
FieldWorks currently displays what word is being parsed and the number of words remaining in the lower left corner of the display. This is very useful feedback for a long running process. Seeing what word is being parsed lets you see which words are taking a long time. Maybe ParseWords could take an optional callback function that is called when a word starts parsing and when it stops? The set of words currently being parsed could be displayed in the lower left corner. |
|
An alternative to passing limits like MaxAlternatives in as an argument to internal functions is to make them be readonly once Morpher has been initialized. In FieldWorks, the Hermit Crab grammar gets recompiled whenever any of the parameters are changed. Are there situations where we want to change MaxAlternatives on the fly? If so, maybe we should make it be an argument to ParseWord(s) rather than a variable in Morpher. |
Done in 4adfd57. |
Agreed, and done in 4adfd57. |
This comment has been minimized.
This comment has been minimized.
…ed progress callbacks. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Public API: adds the public Findings: F1 addressed, F2 withdrawn, F3 accepted (deferred until measured; unverified cost), F4 addressed. Reviewed at 45a3205 |
|
I don't think that you need to throw WordParseProgressException at the end of ParseWords. Just return ordered. ordered has the exceptions stored in WordParseResults. Throwing WordParseProgressException just complicates the call to ParseWords. |
Start here:
src/SIL.Machine.Morphology.HermitCrab/Morpher.cs,ParseWords: the whole public surface, plus the guard that requires a degree-1 Morpher.Status: ready for review; all checks green. #546 (shared template optimization, still open) is not required; this PR is independent of it.
FieldWorks consumer: FLEx's parser worker adopts this through
ParseWordson a degree-1 Morpher, tracked in LT-22885.Morpher.ParseWords(words, maxWordConcurrency = 4, guessRoot = false)parses a batch of words concurrently and returns oneWordParseResultper input position, in input order, duplicates kept. Each result carries materialized analyses, a trace, and any per-word exception, so an invalid shape or resource-limit failure on one word does not discard the others. The single-word API and the constructor default (maxDegreeOfParallelism = 0) are unchanged.What could break existing callers? Nothing opts in implicitly.
ParseWordsthrows unless the Morpher was built withmaxDegreeOfParallelism: 1, which keeps template slots, analysis cascades and synthesis sequential inside each word. One breaking change:MaxAlternativesis now a constructor argument with no setter, fixed for the Morpher's lifetime, so the rules read it directly and parsing writes no shared compiled rule state (AnalysisStratumRule.cs,AnalysisLanguageRule.cs). Callers that set it must move it to the constructor and recompile; FieldWorks'HCParser.csbecomesnew Morpher(m_traceManager, m_language, 0, maxAlternatives).ParseWordsalso takes an optional progress callback, called on the parsing threads when each word starts and finishes.Where to look:
_maxAlternativesfield:AnalysisStratumRule.cs. Pinned byParseWords_CapturesMaxAlternativesPerWord.ParseWords_PreservesInputOrderAndSingleWordAnalysisSets,ParseWords_MatchesSingleWordAnalysisSetsWithCompounding.ParseWords_RequiresSequentialMorpher.ParseWords_TracingRunsOnCallingThreadAndRetainsPerWordTraces.Deliberately not here: FieldWorks adoption (it needs a caller change; the default stays 0); a change to the single-word constructor default; hard or capped words are unmeasured for parity and throughput.
Speed figures are measured wall timings used only as motivation, not a guarantee for this implementation or every Language (details below).
Verification: offline Release build passes with zero warnings or errors; the full HermitCrab NUnit project passes, including the new
ParseWords_*cases; CSharpier checks the five changed files; comment hygiene is clean. Repository-wide./local_check.shand coverage were not run locally.Next: review the
ParseWordssignature and the degree-1 requirement; say if you want the requirement relaxed.Reading this a year from now -- start here
ParseWordsexists so a caller with a queue of pending words can spend parallelism across words instead of inside one word. The measurements, FieldWorks adoption notes and the PanGloss comparison below are the reasoning behind the API shape; the tree only shows the API.Evidence: measured speedups (wall time, secondary)
A local benchmark harness, not part of this PR, measured four concurrent degree-1 words at 3.05x on Sena and 2.20x on Mbugwe versus the degree-0 serial-word default. Those figures include the separate #546 shared template optimization. The same experiment with sharing disabled measured 2.42x and 1.47x.
Caveats: three-pass median wall timings on a shared machine, excluding construction, over small completed cohorts (62 Sena positions; 15 of 42 sampled Mbugwe positions). The harness used one Language/Morpher per worker, whereas this API shares one fixed Morpher. Hard or capped words remain unmeasured; the larger Sena run was unstable under its watchdog. Twenty concurrent words performed worse than four on both primary real-input cohorts, hence the default of 4.
Decisions and why
Deferred: what FieldWorks adoption needs
FieldWorks's parser worker currently submits one word at a time and constructs Morpher without a degree argument (
Src/LexText/ParserCore/HCParser.cs:195in the FieldWorks repo; not checked here). It would need to: construct a degree-1 Morpher; gather a finite batch from its pending queue; callParseWords; map each indexed result, error and trace back to the original request. Language reloads and option changes must wait for the batch to finish; selectors and custom rules must support concurrent calls. FieldWorks is not changed here.🤖 Generated with Claude Code
This change is