Repository navigation
Prune analysis rule outputs that contain no root when no later rule can restore stem material - #572
johnml1135 wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #572 +/- ##
==========================================
+ Coverage 74.57% 74.65% +0.07%
==========================================
Files 453 455 +2
Lines 38342 38596 +254
Branches 5284 5346 +62
==========================================
+ Hits 28593 28813 +220
- Misses 8583 8601 +18
- Partials 1166 1182 +16 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| if (_morpher.TraceManager.IsTracing) | ||
| _morpher.TraceManager.MorphologicalRuleUnapplied(_rule, i, input, outWord); | ||
| output.Add(outWord); | ||
| if (_morpher.MayContainRoot(input, outWord, nameof(AnalysisAffixProcessRule))) |
There was a problem hiding this comment.
Minor: F1. Try-a-Word loses the lexical lookup that tells a user which root is missing. The rule output is traced as MorphologicalRuleUnapplied and then dropped with no event, so its LexicalLookup failure never shows up in the trace. The prune fires in exactly that case: no root matches the stem. Turn the prune off while tracing, as AnalysisStratumRule.Apply does for merging (line 142), or trace the drop. The same applies to AnalysisCompoundingRule.cs:157 and AnalysisRealizationalAffixProcessRule.cs:62.
| /// Controls the root-substring prune of <see cref="MayContainRoot"/>. Shadow evaluates the predicate and | ||
| /// reports through <see cref="RootSubstringShadow"/> without pruning. | ||
| /// </summary> | ||
| internal RootSubstringPruneMode RootSubstringPrune { get; set; } = RootSubstringPruneMode.On; |
There was a problem hiding this comment.
Minor: F2. Callers cannot turn this off. The prune is on by default and changes which analyses come out, but its switch is internal, so FLEx and other netstandard2.0 consumers have no way back if it misprunes a grammar. The comparable switches MergeEquivalentAnalyses (line 100) and PruneDisagreeingCopies (line 117) are public bools. Expose a public bool and keep Shadow internal.
|
|
||
| private Morpher CreateMorpher(RootSubstringPruneMode mode) | ||
| { | ||
| var morpher = new Morpher(TraceManager, Language, maxDegreeOfParallelism: 1) { RootSubstringPrune = mode }; |
There was a problem hiding this comment.
FYI: F3. No test runs the prune in parallel. Every on/off comparison builds its morpher with maxDegreeOfParallelism: 1. The one test that uses the default parallelism (line 281) has the flag off in every stratum, so it never probes. docs/review/hermitcrab.md asks for both modes, and production runs ParallelCombinationRuleCascade with probes running at the same time on shared tries.
There was a problem hiding this comment.
F3 still applies at 01bdf5a: Morpher.MayContainRoot now writes input.RootWitness on the frozen input word, and parallel branches share that word. Every thread writes the same run, so I see no wrong result. But the prune tests still build their morphers with maxDegreeOfParallelism: 1, so none of them runs this write concurrently.
|
|
||
| /// <summary> | ||
| /// Whether any root occurs as a contiguous run of the shape's segments, starting anywhere and ending | ||
| /// anywhere. Stops at the first accepting path. |
There was a problem hiding this comment.
FYI: F4. The doc comment says "Stops at the first accepting path", but that is not what happens. Fst.Transduce with allMatches: false collects every accepting path from the first start position that matches (Traverse(...).ToList(), then Sort, Fst.cs:389-399), and only then stops. Say "stops at the first start position with a match" so later callers do not rely on a cheaper cost than the real one.
This comment has been minimized.
This comment has been minimized.
…an restore stem material A rule output is probed against the root tries only when it removed segments. The probe reuses a root run already found in the input when it still lies inside the output's window of the input's segments, so most probes are skipped; a skipped probe only keeps a word. Closes #562 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
d58a8e8 to
01bdf5a
Compare
| /// Whether any root occurs as a contiguous run of the shape's segments, starting anywhere and ending | ||
| /// anywhere. Stops at the first accepting path. | ||
| /// </summary> | ||
| public bool ContainsRootSubstring(Shape shape) |
There was a problem hiding this comment.
FYI: F5. ContainsRootSubstring is now dead code: since the root-run reuse, Morpher probes only through TryFindRootRun, and nothing in src/ or tests/ calls this method (I grepped both). Drop it. That also settles F4, whose doc comment is on this method.
Public API: none changed (the new members are Findings: F1 open, F2 open, F3 open, F4 open, F5 new. Reviewed at 01bdf5a |
Adds the round-doc section "Combined stack and R2's cost": the full stack against master (work, unification, matcher per grammar), additivity (near-additive on Mbugwe, overlapping on Sena, super-additive on Aweti), R2's root cause (its own probe, low prune rate) and the witness-reuse fix folded into #572 @ 01bdf5a, the 1-4 point residual, and the landing decision: #572 is a draft and lands only on top of #577 and #574. Stress grammars are PR #578 (opt-in, [Explicit], not run in CI). Ledger: row 40 (R2) Outcome records the fold and the landing decision; new row 47 records the combined stack measurement. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Closing. Counting every unit of work as equal, this prune adds 7.7% to Sena's total work, Sena being a real grammar, and wins only on a synthetic early-lexicon grammar. We tried to measure how much a unification call costs relative to an event so the trade-off could be weighed properly, but the result was not reliable. The record is in #490. Comment drafted by Claude (AI) for the maintainer. |
Quick summary
Land only on top of #577 and #574, not on its own. Alone, this PR does more unification and matcher work than master on 3 of the 4 grammars it affects (Sena, conformance, the Bantu stress grammar); it saves events there, and saves on every count only on the early-lexicon fixture. Below, "less work" means analysis events plus synthesis steps, not unification or matcher work. In the full stack (#577, #576, #574 and this) it costs 1-4 points of unification and matcher work against the stack without it on conformance, Sena and the Bantu stress grammar.
HermitCrab analysis now drops a rule output that contains no root as a contiguous run of segments, in strata where nothing can later restore stem material. Against master it does 17.29% less work (analysis events plus synthesis steps) on Sena and 67.72% less on a synthetic early-lexicon agglutinative fixture, with identical parses. It costs unification and matcher work, because the probe is a trie walk: Sena does 9.05% more unification calls and 8.05% more matcher work than master, conformance 1.00% more and 1.14% more, the Bantu template stress grammar 37.48% more and 41.56% more; the early-lexicon fixture does 58.20% less unification and 59.42% less matcher work. Indonesian, Mbugwe, Amharic and Aweti are unaffected because their flag is off, so they make no probes. A probe is skipped when the output keeps at least as many segments as its input, and, new in this version, when a root run already found in the input still lies inside the output's window of the input's segments; a skipped probe only keeps a word. There is no adaptive off switch. Traces (Try-a-Word) lose the pruned subtree. Pruned analyses no longer count toward
MaxAlternatives.Where to look
RootSubstringPruneStratum(new file): a stratum is on only if every rule of it and of deeper strata copies all LHS parts once, unmodified, in order and adjacent, no deeper stratum has phonological rules, and no lexical-pattern root exists;RootSubstringPruneTestshas one test per condition and each goes red when its condition is removed.AnalysisAffixProcessRule,AnalysisRealizationalAffixProcessRule,AnalysisCompoundingRuleskip onlyoutput.Add; trace callbacks are unchanged.RootAllomorphTrie.ContainsRootSubstring: one unanchored, end-anchor-off transduction per trie.Morpher.MayContainRootskips the probe when the output has at least as many segments as its input in the same stratum, or when a root run found in the input (Word.RootWitness, found once per input) lies inside the output's window of the input's segments (Word.RootWindowStart, set by the affix and compounding-head analysis rules only when the prune is active for the stratum; neither field is copied byClone). Skipping only keeps a word; a stale witness can never prune. The probe isRootAllomorphTrie.TryFindRootRun: the same arcs as before, tried per start segment, so it also says where the run is. The probe count is exposed through a null-by-defaultRootSubstringProbedhook, so the hot path has no shared counter.RootSubstringPrune = Off(see below).Deliberately not included
ModifyFromInputor segment-restoring rules (Indonesian, Mbugwe, Amharic, Aweti): a subsequence or restoration-class fallback would be a separate change.Validation
dotnet test tests/SIL.Machine.Morphology.HermitCrab.Tests -c Release-- 164 passed, 0 failed (origin/master: 151 passed, 0 failed); three new tests inRootSubstringPruneTests: probe count and prune decisions with root-run reuse, the witness found once and copied, and a stale witness that only skips.dotnet csharpier check .-- clean.pwsh -NoProfile -File ./scripts/comment-hygiene.ps1-- clean../local_check.sh --agent-strictwas not completed: it stops at the SentencePiece cmake build.dotnet run --project src/SIL.Machine.Morphology.HermitCrab.Conformance -- --fixtures conformance --include-pathological) -- 146 passed, 4 failed on both; per-fixture verdicts identical (the 4 are the truncate-morphotactic fixtures).Before the root-run reuse the same arms were: conformance 12.15% more unification and 14.97% more matcher work, Bantu stress 67.53% more and 70.76% more, early-lexicon 58.02% less and 59.58% less, Sena 8.47% more and 7.50% more (Sena is 0.58 and 0.55 points worse with reuse when this branch runs alone; in the combined stack with the other analysis PRs it is better). Events are unchanged by the reuse, as it only skips probes.
Parse parity: per-word signatures equal to master on every completed word in all arms (0 differences over 1,796 completed words: Mbugwe 40, Sena 51, Aweti 99, Amharic 24, Indonesian 70, conformance 1,452, stress 60 x 3).
Where the prune pays on events it can still cost more unification and matcher work than master. Measured with the other analysis PRs stacked (not in this PR), R2 with the reuse still does 1 to 4 points more unification and matcher work than the same stack without R2 on conformance (1.03 and 1.25 points), Sena (1.52 points of unification) and the Bantu stress grammar (3.71 points of unification), while it saves events (Sena 4.74 points, conformance 0.20, Bantu stress 0.50).
Where the prune costs more work than master
The probe is a trie walk, one feature-structure unification per arc, and a pass still costs the arcs up to the first accepting path. Alone against master, the prune saves events everywhere the flag is on but does 9.05% more unification calls on Sena, 1.00% more on conformance and 37.48% more on the Bantu template stress grammar (where the root lexicon is large enough that late prunes save little). It does 58.20% less on the early-lexicon fixture, which is synthetic and built to show the best case. The other four real grammars gain nothing and lose nothing, because the flag is off. Root-run reuse removed most of the earlier overhead (conformance 12.15% to 1.00% more, Bantu stress 67.53% to 37.48% more) but not the residual: the single input probe per stratum entry, the decisive probes, and probes where the stem root itself is cut and another root must be searched. I found no sound grammar-independent rule that removes it without a prune-rate or lexicon-size gate, and a gate was tried and rejected (it fits only the conformance toy lexicons). The residual is stated plainly: R2 still does 1 to 4 points more unification and matcher work than no R2 on conformance, Sena and the Bantu stress grammar when stacked with the other analysis PRs, while it saves events.
Traces
Try-a-Word and other trace output lose the pruned subtree: a rule output that is dropped never reaches its later rules, so those nodes do not appear in the trace.
Issue / porting context
Closes #562
Why the 12 direct-rule tests set RootSubstringPrune = Off
Twelve existing tests (one in
AnalysisSyntacticFeatureStructTests, eleven inPruneDisagreeingCopiesTeststhrough itsCreateMorpherhelper) failed with the prune on. They are not parses. Each builds aMorpher, then constructsAnalysisAffixProcessRulefor a rule that was never added to any stratum, and applies it directly to a hand-builtWordwhose shape is arbitrary and holds no lexicon root (for examplezimui, or a copy-agreement shape). The morpher computes its stratum flags in its constructor from the rules the language holds, so for these tests the flag is computed without the rule under test (the copy-twice rule would have turned it off). The prune then correctly sees a shape with no root and drops the output, which empties the list the test wants to inspect.A real
Morpher.ParseWordcannot reach that state: every rule it applies belongs to a stratum, the flag is computed with that rule included, and the word comes from the surface form through the cascade. OnlyAnalysisSyntacticFeatureStructTests.AnalysisAffixProcessRule_CategoryChangeChain_RequiredOverridesAccumulatedPosand every test inPruneDisagreeingCopiesTests(itsCreateMorphersetsOfffor the whole file) run with the prune off; the other tests inAnalysisSyntacticFeatureStructTestskeep it on and pass. Whether the prune can fire on a real parse these tests represent: only on a branch where no lookup below the output could succeed, and, for the copy rules inPruneDisagreeingCopiesTests, never, because reduplication turns the flag off for the stratum.Reading this a year from now
Lookup is exact, anchored at both ends. Below a rule output N, every word analysis can produce is a contiguous run of N's segment nodes with feature structures at least as specific, provided all rules at and below N copy their LHS parts adjacently and in order (so only removal happens), no deeper stratum has phonological rules (those can change segments between N and the lookup), and there are no lexical-pattern roots (
guessRootmatches patterns against analyses, so analyses must not be removed). An exact root match anywhere below N is then a substring match in N.LexEntrySelectoronly removes roots, so searching all roots is an over-approximation. A compound passes the test because it contains its head root. The probe is oneFst.Transduceper non-empty trie of the stratum and deeper strata, with start anchor off and end anchor off, returning on the first accept; a root with no segments makes the test always true.Pruned analyses no longer count toward
MaxAlternatives, so a parse that used to throwMaxAlternativesExceededExceptioncan now complete; the reverse cannot happen.Paths not taken
A reachability summary (ledger row 27) was closed for 0-5% prunable internal nodes; this prune is keyed on the shape instead. A gapped (subsequence) fallback for infix and reduplication strata, and wildcard classes for restoring rules, would enable Indonesian, Mbugwe, Amharic and Aweti, whose census ceilings are 29.6%, 38.5%, 13.6% and 14.8%; they are separate changes.
🤖 Generated with Claude Code
This change is