Repository navigation
Skip synthesis of candidates whose MPR features or stem names fail - #570
johnml1135 wants to merge 1 commit into
Conversation
After lexical lookup, each alternative is replayed against its root without shapes and dropped when a rule's recorded allomorph requires MPR features the root and earlier trail outputs cannot supply, excludes features the root certainly has, or the rule requires a different stem name. The walk stops at the first violation, runs only when some rule carries such a constraint, and keeps nothing between candidates. Family roots, compounding entries and reachable Overwrite groups disable the unsafe checks. Dropped candidates emit no synthesis trace events. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #570 +/- ##
==========================================
+ Coverage 74.57% 75.00% +0.43%
==========================================
Files 453 454 +1
Lines 38342 38495 +153
Branches 5284 5317 +33
==========================================
+ Hits 28593 28874 +281
+ Misses 8583 8469 -114
+ Partials 1166 1152 -14 ☔ 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}
| alternativeCount++; | ||
| if (MaxAlternatives > 0 && alternativeCount > MaxAlternatives) | ||
| throw new MaxAlternativesExceededException("MaxAlternatives exceeded"); | ||
| if (IsPrunedByLookupReplay(alternative)) |
There was a problem hiding this comment.
Minor: F1. With tracing on, a pruned candidate leaves an empty trace node. LexicalLookup (Morpher.cs:466) has already emitted SynthesizeWord for it, but the RequiredStemName/RequiredMprFeatures events that explained the failure are gone. Try-a-Word users see a lookup that ends with no reason. LookupReplayMprTests.cs:42-43 confirms it: they count fewer synthesis events with pruning on under IsTracing = true. The precedent is AnalysisStratumRule.cs:142, which skips merging while tracing; skipping pruning the same way would keep traces unchanged. The parallel loop at line 415 needs the same fix.
| if (upper == null) | ||
| { | ||
| work++; | ||
| upper = lower.Clone(); |
There was a problem hiding this comment.
FYI: F2. upper is a new clone of lower plus extra for every trail entry that has required features, even when extra is still the empty _realizationalOut. In that case lower already is the superset. Using it directly saves one allocation and one work unit per entry. That matters here because the PR's own table puts work/saved at about 1.0 for Aweti and Mbugwe.
| [Test] | ||
| public void NullRecordedAllomorph_IsPrunedOnlyWhenEveryAllomorphFails() | ||
| { | ||
| // Guessed (null) allomorphs happen when the analysis came from a rule whose allomorph is unconstrained; |
There was a problem hiding this comment.
FYI: F3. This comment gives the wrong cause for a null allomorph. AnalysisAffixProcessRule.cs:92 always records _rule.Allomorphs[i], so ParseWord never hands the replay a null allomorph for an affix rule. Nulls come only from GenerateWords/PermuteRules, and that path never calls IsFeasible. The null branches at LookupReplay.cs:145-155 and 167-177 are defensive code. Say that here, so nobody tunes them for a parse case that cannot happen.
Public API, target framework, package, machine.py parity: None verified. Every new member is Findings: F1 new, F2 new, F3 new. Reviewed at c6e2bbc |
Quick summary
After lexical lookup,
Morpherdrops an alternative before synthesis when a rule needs a stem name or MPR features its root and trail cannot supply. Real grammars come out about break-even in total work (Aweti +0.07%, Mbugwe 0.00%), conformance and inflection-class fixtures lose slightly (-0.45%, -4.77%), and concord-heavy grammars gain 18%. No parse is lost. Analysis, lookup and public API are untouched.Where to look
LookupReplay.IsMprFeasible;LookupReplayMprTests,LookupReplayOutputsTests(Any/Overwrite groups, realizational and compounding outputs, parallel path).MaxAlternativescount, per alternative --Morpher.IsPrunedByLookupReplay.Deliberately not included
LookupReplayMode,LookupReplayVerdictandLookupReplayWorkstay because the tests read per-alternative verdicts and work.Validation
dotnet test tests/SIL.Machine.Morphology.HermitCrab.Tests -c Release-- 170 passed (origin/master151)dotnet csharpier check .andpwsh ./scripts/comment-hygiene.ps1-- cleanEvidence and caveats
truncate-morphotactic*fixtures, which also differ between two runs with the filter off.ClassGatedSlots_PruneMostWrongClassCandidates...reproduces the effect as a unit test.Measured work, in steps
Failure-statistics harness (research/failure-reason-stats), one Release build, serial, 60 s per word, words aborted in any arm dropped. Replay work counts one unit per feature-set match, union or copy; units and steps are not shown to cost the same. Analysis events and lookup candidates are unchanged in every row. Net total-work is (steps saved - replay work) over synthesis steps plus analysis events, counting a unit as a step.
The two stress rows are synthetic fixtures not in this PR.
Memory: two MPR feature sets per grammar (realizational and compounding outputs), built at construction, and a clone of the root's set only when a trail entry adds features.
Deferred, and what would unblock it
🤖 Generated with Claude Code
This change is