diff --git a/src/SIL.Machine.Morphology.HermitCrab/AnalysisStateKey.cs b/src/SIL.Machine.Morphology.HermitCrab/AnalysisStateKey.cs index fe48f977..535593cd 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/AnalysisStateKey.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/AnalysisStateKey.cs @@ -87,10 +87,7 @@ public bool Equals(AnalysisStateKey other) return false; if (!_shape.ValueEquals(other._shape)) return false; - if ( - !_syntacticFS.ValueEquals(other._syntacticFS) - || !_realizationalFS.ValueEquals(other._realizationalFS) - ) + if (!_syntacticFS.ValueEquals(other._syntacticFS) || !_realizationalFS.ValueEquals(other._realizationalFS)) return false; return RuleCountsEqual(_ruleCounts, other._ruleCounts); } diff --git a/src/SIL.Machine.Morphology.HermitCrab/GrammarAnalyzer.cs b/src/SIL.Machine.Morphology.HermitCrab/GrammarAnalyzer.cs index f6428230..d1960788 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/GrammarAnalyzer.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/GrammarAnalyzer.cs @@ -12,15 +12,16 @@ namespace SIL.Machine.Morphology.HermitCrab /// "Gate B" -- Gate A, a mirror-image synthesis-side bound, was attempted and reverted; see the note /// in ). The bound is a deliberately loose over-approximation /// -- summed across every rule's own already-declared reapplication limit - /// (, - /// ), never estimated -- so it can prune a candidate only - /// when NO combination of rules in the grammar could ever produce something that long, regardless of - /// which specific root or derivation path is under consideration. Returns null (meaning "no admissible - /// bound, gate off") the moment any rule's shape falls outside what this class knows how to measure - /// exactly (quantifiers/groups/alternations in a phonological Lhs/Rhs, or a compounding rule present - /// at all, since compounding combines multiple full root lengths rather than adding a bounded affix) - /// -- per the plan's own rule: skipping only costs pruning opportunity, an admissible bound must never - /// be guessed. + /// (), + /// never estimated -- so it can prune a candidate only when NO combination of rules in the grammar + /// could ever produce something that long, regardless of which specific root or derivation path is + /// under consideration. Returns null (meaning "no admissible bound, gate off") the moment any rule's + /// shape falls outside what this class knows how to measure exactly (quantifiers/groups/alternations + /// in a phonological Lhs/Rhs, any phonological rewrite subrule whose Lhs and Rhs segment counts differ + /// -- see LT-22613, and the remarks on 's caller below -- or a + /// compounding rule present at all, since compounding combines multiple full root lengths rather than + /// adding a bounded affix) -- per the plan's own rule: skipping only costs pruning opportunity, an + /// admissible bound must never be guessed. /// public static class GrammarAnalyzer { @@ -29,23 +30,25 @@ public static class GrammarAnalyzer /// represent: the longest root allomorph in the lexicon, plus every affix/realizational rule's own /// maximum possible net insertion (its allomorphs' / /// actions, summed and multiplied by - /// ), - /// plus every phonological deletion-type subrule's maximum possible net restoration. Null if any - /// rule in the grammar can't be measured this way (see class remarks). + /// ). + /// Null if any rule in the grammar can't be measured this way (see class remarks) -- in + /// particular, every phonological rewrite subrule in the grammar must leave Lhs and Rhs segment + /// counts equal (a pure feature-changing rule, handled analysis-side by + /// FeatureAnalysisRewriteRuleSpec, which mutates matched segments in place and never + /// changes the shape's length). Any subrule where they differ -- epenthesis/expansion (Rhs longer) + /// or deletion/coalescence (Lhs longer) alike -- is unapplied analysis-side by + /// EpenthesisAnalysisRewriteRuleSpec/NarrowAnalysisRewriteRuleSpec, both of which + /// grow the candidate shape by inserting Lhs.Count new (Optional, not Deleted) nodes per + /// match site regardless of Rhs.Count, while leaving the matched Rhs-shaped region in place + /// (also not Deleted). counts every non-Deleted + /// segment, so real per-site growth is always exactly Lhs.Count -- never the naively + /// expected Lhs.Count - Rhs.Count, which undercounts by Rhs.Count whenever + /// Rhs.Count > 0 (LT-22613: first found via epenthesis pruning a valid analysis as + /// "too long" on the default non-tracing path while the tracing path, which bypasses this gate, + /// parsed it correctly; the same undercount also applies to ordinary deletion/coalescence rules, + /// which is why the bail-out is on any count mismatch, not just Rhs longer than Lhs). /// - /// - /// The phonological term is compounding, not additive: AnalysisRewriteRule's Deletion - /// reapply loop runs + 1 passes, and each pass is a - /// SimultaneousPhonologicalPatternRule sweep that can restore EVERY non-overlapping match - /// site in the current shape at once, not just one -- a real case (RewriteRuleTests - /// .MultipleDeletionRules: an 8-segment root deletes two independent "ii" clusters down to a - /// 4-segment surface form in one pass) needs more than "count of subrules" restored segments per - /// pass. Bounding the number of sites by the current running length (itself already an - /// over-approximation of the true pre-phonology length at this point) keeps this sound: real growth - /// can never exceed runningLength * subruleDelta per pass, since a simultaneous sweep cannot - /// match more sites than there are segments to match against. - /// - public static int? ComputeMaxAnalysisLength(Language language, int deletionReapplications) + public static int? ComputeMaxAnalysisLength(Language language) { int bound = 0; foreach (Stratum stratum in language.Strata) @@ -53,7 +56,11 @@ public static class GrammarAnalyzer if (stratum.MorphologicalRules.OfType().Any()) return null; - int longestRoot = stratum.Entries.SelectMany(e => e.Allomorphs).Select(SegmentCount).DefaultIfEmpty(0).Max(); + int longestRoot = stratum + .Entries.SelectMany(e => e.Allomorphs) + .Select(SegmentCount) + .DefaultIfEmpty(0) + .Max(); bound += longestRoot; foreach (AffixProcessRule rule in stratum.MorphologicalRules.OfType()) @@ -64,7 +71,6 @@ RealizationalAffixProcessRule rule in stratum.MorphologicalRules.OfType()) { if (!TryGetFlatSegmentCount(rule.Lhs, out int lhsCount)) @@ -73,12 +79,13 @@ RealizationalAffixProcessRule rule in stratum.MorphologicalRules.OfType rhsCount) - phonoGrowthRate += lhsCount - rhsCount; + // Any subrule where Lhs and Rhs segment counts differ: no admissible bound + // (LT-22613; see this method's doc comment for the full mechanism). Only a + // count-preserving (pure feature-changing) subrule is safe to ignore here. + if (lhsCount != rhsCount) + return null; } } - for (int pass = 0; pass < deletionReapplications + 1 && phonoGrowthRate > 0; pass++) - bound += bound * phonoGrowthRate; } return bound; } diff --git a/src/SIL.Machine.Morphology.HermitCrab/MemoizedCombinationRuleCascade.cs b/src/SIL.Machine.Morphology.HermitCrab/MemoizedCombinationRuleCascade.cs index 7dc392b7..e3b18f57 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/MemoizedCombinationRuleCascade.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/MemoizedCombinationRuleCascade.cs @@ -75,7 +75,11 @@ private List ApplyRules(Word input, HashSet output) var replayed = new List(entry.Results.Count); foreach (Word storedResult in entry.Results) { - Word replay = storedResult.ReplayOnto(input, entry.MruleTrailPrefixLength, entry.NonHeadPrefixLength); + Word replay = storedResult.ReplayOnto( + input, + entry.MruleTrailPrefixLength, + entry.NonHeadPrefixLength + ); output.Add(replay); replayed.Add(replay); } diff --git a/src/SIL.Machine.Morphology.HermitCrab/Morpher.cs b/src/SIL.Machine.Morphology.HermitCrab/Morpher.cs index 22295305..86189ba0 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/Morpher.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/Morpher.cs @@ -130,10 +130,10 @@ public ITraceManager TraceManager /// "Gate B") -- auto-derived from the grammar (: /// the longest lexicon root plus every rule's own maximum possible insertion) unless explicitly set. /// Setting this (including to null, which disables the gate entirely) overrides the - /// auto-derived value; re-derived fresh from the current grammar and - /// on every read otherwise, so it never goes stale if either changes after construction. Auto-derives - /// to null (gate off) when the grammar contains a compounding rule or a phonological rule shape - /// this analysis can't measure exactly -- see 's remarks. + /// auto-derived value; re-derived fresh from the current grammar on every read otherwise, so it + /// never goes stale if the grammar changes after construction. Auto-derives to null (gate + /// off) when the grammar contains a compounding rule or a phonological rule shape this analysis + /// can't measure exactly -- see 's remarks. /// public int? MaxAnalysisLength { @@ -141,7 +141,7 @@ public int? MaxAnalysisLength { return _maxAnalysisLengthOverrideSet ? _maxAnalysisLengthOverride - : GrammarAnalyzer.ComputeMaxAnalysisLength(_lang, DeletionReapplications); + : GrammarAnalyzer.ComputeMaxAnalysisLength(_lang); } set { diff --git a/tests/SIL.Machine.Morphology.HermitCrab.Tests/MorpherTests.cs b/tests/SIL.Machine.Morphology.HermitCrab.Tests/MorpherTests.cs index 1f5b6c24..7d9f848b 100644 --- a/tests/SIL.Machine.Morphology.HermitCrab.Tests/MorpherTests.cs +++ b/tests/SIL.Machine.Morphology.HermitCrab.Tests/MorpherTests.cs @@ -775,9 +775,7 @@ public void EnableLexicalGating_MatchesDisabled_SimpleAffixGrammar() List onResult = gateOn.ParseWord(word).ToList(); Assert.That( onResult.Select(WordResultSignature).OrderBy(s => s, System.StringComparer.Ordinal), - Is.EqualTo( - offResult.Select(WordResultSignature).OrderBy(s => s, System.StringComparer.Ordinal) - ), + Is.EqualTo(offResult.Select(WordResultSignature).OrderBy(s => s, System.StringComparer.Ordinal)), $"lexical-gate-on parse of '{word}' must match gate-off parse" ); } @@ -852,4 +850,72 @@ public void IsEdgeStripperQualified_ReturnsFalse_ForInfixation() Allophonic.MorphologicalRules.Remove(infix); } } + + [Test] + public void ComputeMaxAnalysisLength_ReturnsNull_ForInsertionRewriteRule() + { + // LT-22613: an insertion-type rewrite subrule (Rhs longer than Lhs -- epenthesis when Lhs is + // empty) makes the analysis length bound inadmissible: its unapplication marks the inserted + // surface segments OPTIONAL rather than removing them (EpenthesisAnalysisRewriteRuleSpec/ + // NarrowAnalysisRewriteRuleSpec), so a candidate's segment count at AnalysisStratumRule's gate + // no longer bounds the underlying length the candidate could still justify. Per this class's + // own ground rule, that means "no admissible bound" (null, gate off), not a guessed bound. + var highVowel = FeatureStruct + .New(Language.PhonologicalFeatureSystem) + .Symbol(HCFeatureSystem.Segment) + .Symbol("cons-") + .Symbol("voc+") + .Symbol("high+") + .Value; + var epenthesis = new RewriteRule { Name = "epenthesis", ApplicationMode = RewriteApplicationMode.Simultaneous }; + epenthesis.Subrules.Add( + new RewriteSubrule + { + Rhs = Pattern.New().Annotation(highVowel).Value, + LeftEnvironment = Pattern.New().Annotation(highVowel).Value, + } + ); + Allophonic.PhonologicalRules.Add(epenthesis); + try + { + Assert.That(GrammarAnalyzer.ComputeMaxAnalysisLength(Language), Is.Null); + } + finally + { + Allophonic.PhonologicalRules.Remove(epenthesis); + } + } + + [Test] + public void ComputeMaxAnalysisLength_ReturnsNull_ForDeletionRewriteRule() + { + // LT-22613 follow-up: a deletion/coalescence-type rewrite subrule (Lhs longer than Rhs) is + // unapplied by the same NarrowAnalysisRewriteRuleSpec as expansion, which always inserts + // Lhs.Count new (Optional, not Deleted) nodes per match site and leaves the matched Rhs-shaped + // region in place (also not Deleted) -- so real per-site growth is Lhs.Count, not the + // Lhs.Count - Rhs.Count this method used to budget. That undercounts by Rhs.Count whenever + // Rhs.Count > 0, making the bound inadmissible here too, not just for insertion. + var vowel = FeatureStruct + .New(Language.PhonologicalFeatureSystem) + .Symbol(HCFeatureSystem.Segment) + .Symbol("cons-") + .Symbol("voc+") + .Value; + var coalescence = new RewriteRule + { + Name = "coalescence", + ApplicationMode = RewriteApplicationMode.Simultaneous, + Lhs = Pattern.New().Annotation(vowel).Annotation(vowel).Value, + }; + coalescence.Subrules.Add(new RewriteSubrule { Rhs = Pattern.New().Annotation(vowel).Value }); + Allophonic.PhonologicalRules.Add(coalescence); + try + { + Assert.That(GrammarAnalyzer.ComputeMaxAnalysisLength(Language), Is.Null); + } + finally + { + Allophonic.PhonologicalRules.Remove(coalescence); + } + } } diff --git a/tests/SIL.Machine.Morphology.HermitCrab.Tests/PhonologicalRules/RewriteRuleTests.cs b/tests/SIL.Machine.Morphology.HermitCrab.Tests/PhonologicalRules/RewriteRuleTests.cs index 68913bba..b7dae6d2 100644 --- a/tests/SIL.Machine.Morphology.HermitCrab.Tests/PhonologicalRules/RewriteRuleTests.cs +++ b/tests/SIL.Machine.Morphology.HermitCrab.Tests/PhonologicalRules/RewriteRuleTests.cs @@ -1341,6 +1341,115 @@ public void EpenthesisRules() AssertMorphsEqual(morpher.ParseWord("butubu"), "25"); } + [Test] + public void EpenthesisRuleWithMinimalLexicon() + { + // LT-22613 regression: the default (non-tracing) parse path must give the same answer as the + // traced path. EpenthesisRules above only passes because this fixture's large lexicon makes + // GrammarAnalyzer.ComputeMaxAnalysisLength's auto-derived bound big enough to hide the bug: an + // epenthesis rule's unapplication marks the inserted segments OPTIONAL (it never removes them, + // see EpenthesisAnalysisRewriteRuleSpec.Unapply), so a surface form that outgrew the longest + // root via epenthesis still counts all its segments at AnalysisStratumRule's length gate and + // was pruned as "unreachable" -- 0 parses non-traced, 1 parse traced. This grammar is + // EpenthesisRules sub-case (1) with the lexicon cut down to root 19 alone ("b+ubu", 4 + // segments), so surface "buibui" (6 segments) exceeds any bound derived from the lexicon. + var highVowel = FeatureStruct + .New(Language.PhonologicalFeatureSystem) + .Symbol(HCFeatureSystem.Segment) + .Symbol("cons-") + .Symbol("voc+") + .Symbol("high+") + .Value; + var highFrontUnrndVowel = FeatureStruct + .New(Language.PhonologicalFeatureSystem) + .Symbol(HCFeatureSystem.Segment) + .Symbol("cons-") + .Symbol("voc+") + .Symbol("high+") + .Symbol("back-") + .Symbol("round-") + .Value; + + var morphophonemic = new Stratum(Table3) + { + Name = "Morphophonemic", + MorphologicalRuleOrder = MorphologicalRuleOrder.Unordered, + }; + var allophonic = new Stratum(Table1) + { + Name = "Allophonic", + MorphologicalRuleOrder = MorphologicalRuleOrder.Unordered, + }; + var surface = new Stratum(Table1) + { + Name = "Surface", + MorphologicalRuleOrder = MorphologicalRuleOrder.Unordered, + }; + + var entry = new LexEntry + { + Id = "19", + SyntacticFeatureStruct = FeatureStruct.New(Language.SyntacticFeatureSystem).Symbol("N").Value, + Gloss = "19", + }; + entry.Allomorphs.Add(new RootAllomorph(new Segments(Table3, "b+ubu", true))); + morphophonemic.Entries.Add(entry); + + var rule4 = new RewriteRule { Name = "rule4", ApplicationMode = RewriteApplicationMode.Simultaneous }; + allophonic.PhonologicalRules.Add(rule4); + rule4.Subrules.Add( + new RewriteSubrule + { + Rhs = Pattern.New().Annotation(highFrontUnrndVowel).Value, + LeftEnvironment = Pattern.New().Annotation(highVowel).Value, + } + ); + + var lang = new Language + { + Name = "EpenthesisMinimal", + PhonologicalFeatureSystem = Language.PhonologicalFeatureSystem, + SyntacticFeatureSystem = Language.SyntacticFeatureSystem, + Strata = { morphophonemic, allophonic, surface }, + }; + + var morpher = new Morpher(TraceManager, lang); + AssertMorphsEqual(morpher.ParseWord("buibui"), "19"); + + // The same over-prune is not Simultaneous-specific -- any insertion-type subrule's + // unapplication leaves the inserted segments in the shape as optional nodes. This is + // EpenthesisRules's Iterative cons_highBackRndVowel sub-case, same minimal lexicon. + var highBackRndVowel = FeatureStruct + .New(Language.PhonologicalFeatureSystem) + .Symbol(HCFeatureSystem.Segment) + .Symbol("cons-") + .Symbol("voc+") + .Symbol("high+") + .Symbol("back+") + .Symbol("round+") + .Value; + var cons = FeatureStruct + .New(Language.PhonologicalFeatureSystem) + .Symbol(HCFeatureSystem.Segment) + .Symbol("cons+") + .Symbol("voc-") + .Value; + + rule4.ApplicationMode = RewriteApplicationMode.Iterative; + rule4.Subrules.Clear(); + rule4.Subrules.Add( + new RewriteSubrule + { + Rhs = Pattern.New().Annotation(highFrontUnrndVowel).Value, + LeftEnvironment = Pattern.New().Annotation(cons).Value, + RightEnvironment = Pattern.New().Annotation(highBackRndVowel).Value, + } + ); + + morpher = new Morpher(TraceManager, lang); + AssertMorphsEqual(morpher.ParseWord("biubiu"), "19"); + } + [Test] public void DeletionRules() {