From 81c7364dfd08f75bc6df109c21cb1d2f511f9b87 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Fri, 9 Oct 2026 08:19:59 -0700 Subject: [PATCH 1/7] Fix a performance bug in NarrowAnalysisRewriteRuleSpec --- .../PhonologicalRules/AnalysisRewriteRule.cs | 5 ++-- .../NarrowAnalysisRewriteRuleSpec.cs | 30 +++++++++++++++---- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/AnalysisRewriteRule.cs b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/AnalysisRewriteRule.cs index 95eacbf73..da762fae6 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/AnalysisRewriteRule.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/AnalysisRewriteRule.cs @@ -85,8 +85,9 @@ Constraint constraint in sr.Rhs.Children.Cast< { // NarrowAnalysisRewriteRuleSpec works for expansion, too. ruleSpec = new NarrowAnalysisRewriteRuleSpec(settings, _rule.Lhs, sr); - mode = RewriteApplicationMode.Simultaneous; - reapplyType = ReapplyType.Deletion; + // Expansion is like epenthesis. + if (_rule.ApplicationMode == RewriteApplicationMode.Simultaneous) + reapplyType = ReapplyType.SelfOpaquing; } Debug.Assert(ruleSpec != null); diff --git a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs index 9d913b06c..a8365934b 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs @@ -40,6 +40,9 @@ RewriteSubrule subrule private void Unapply(Match targetMatch, Range range, VariableBindings varBindings) { ShapeNode curNode = IsTargetEmpty ? range.Start : range.End; + ShapeNode targetNode = range.Start; + int targetPos = 0; + bool expanding = _analysisRhs.Children.Count > _targetCount; foreach ( Constraint constraint in _analysisRhs.Children.CastToConstraints( "The target (left-hand side) of a rewrite rule" @@ -49,14 +52,31 @@ Constraint constraint in _analysisRhs.Children.CastToConstraint FeatureStruct fs = constraint.FeatureStruct.Clone(); if (varBindings != null) fs.ReplaceVariables(varBindings); - curNode = targetMatch.Input.Shape.AddAfter(curNode, fs, true); + if (targetPos < _targetCount) + { + // Union fs into the existing node's feature structure. + targetNode.Annotation.FeatureStruct.Union(fs); + if (expanding) + targetNode.SetDirty(true); + targetNode = targetNode.Next; + targetPos++; + } + else + { + // Add an optional node for fs. + curNode = targetMatch.Input.Shape.AddAfter(curNode, fs, true); + if (expanding) + curNode.SetDirty(true); + } } - curNode = range.Start; - for (int i = 0; i < _targetCount; i++) + // Make the rest of the existing nodes optional. + for (int i = targetPos; i < _targetCount; i++) { - curNode.Annotation.Optional = true; - curNode = curNode.Next; + targetNode.Annotation.Optional = true; + if (expanding) + targetNode.SetDirty(true); + targetNode = targetNode.Next; } } } From 63c5d72aefbf765a6123457e4476f0aa07103048 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Mon, 12 Oct 2026 08:55:58 -0700 Subject: [PATCH 2/7] Revert changes to expansion; deal with boundaries --- .../PhonologicalRules/AnalysisRewriteRule.cs | 5 ++-- .../NarrowAnalysisRewriteRuleSpec.cs | 16 +++++----- .../PhonologicalRules/RewriteRuleTests.cs | 29 +++++++++++++++++++ 3 files changed, 39 insertions(+), 11 deletions(-) diff --git a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/AnalysisRewriteRule.cs b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/AnalysisRewriteRule.cs index da762fae6..95eacbf73 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/AnalysisRewriteRule.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/AnalysisRewriteRule.cs @@ -85,9 +85,8 @@ Constraint constraint in sr.Rhs.Children.Cast< { // NarrowAnalysisRewriteRuleSpec works for expansion, too. ruleSpec = new NarrowAnalysisRewriteRuleSpec(settings, _rule.Lhs, sr); - // Expansion is like epenthesis. - if (_rule.ApplicationMode == RewriteApplicationMode.Simultaneous) - reapplyType = ReapplyType.SelfOpaquing; + mode = RewriteApplicationMode.Simultaneous; + reapplyType = ReapplyType.Deletion; } Debug.Assert(ruleSpec != null); diff --git a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs index a8365934b..67e23ddf2 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs @@ -42,7 +42,7 @@ private void Unapply(Match targetMatch, Range range, ShapeNode curNode = IsTargetEmpty ? range.Start : range.End; ShapeNode targetNode = range.Start; int targetPos = 0; - bool expanding = _analysisRhs.Children.Count > _targetCount; + bool prune = true; foreach ( Constraint constraint in _analysisRhs.Children.CastToConstraints( "The target (left-hand side) of a rewrite rule" @@ -52,12 +52,15 @@ Constraint constraint in _analysisRhs.Children.CastToConstraint FeatureStruct fs = constraint.FeatureStruct.Clone(); if (varBindings != null) fs.ReplaceVariables(varBindings); - if (targetPos < _targetCount) + if ( + prune + && targetPos < _targetCount + && constraint.Type() == HCFeatureSystem.Segment + && targetNode.Type() == HCFeatureSystem.Segment + ) { // Union fs into the existing node's feature structure. targetNode.Annotation.FeatureStruct.Union(fs); - if (expanding) - targetNode.SetDirty(true); targetNode = targetNode.Next; targetPos++; } @@ -65,8 +68,7 @@ Constraint constraint in _analysisRhs.Children.CastToConstraint { // Add an optional node for fs. curNode = targetMatch.Input.Shape.AddAfter(curNode, fs, true); - if (expanding) - curNode.SetDirty(true); + prune = false; } } @@ -74,8 +76,6 @@ Constraint constraint in _analysisRhs.Children.CastToConstraint for (int i = targetPos; i < _targetCount; i++) { targetNode.Annotation.Optional = true; - if (expanding) - targetNode.SetDirty(true); targetNode = targetNode.Next; } } diff --git a/tests/SIL.Machine.Morphology.HermitCrab.Tests/PhonologicalRules/RewriteRuleTests.cs b/tests/SIL.Machine.Morphology.HermitCrab.Tests/PhonologicalRules/RewriteRuleTests.cs index 5e0bf867b..7a11afcfa 100644 --- a/tests/SIL.Machine.Morphology.HermitCrab.Tests/PhonologicalRules/RewriteRuleTests.cs +++ b/tests/SIL.Machine.Morphology.HermitCrab.Tests/PhonologicalRules/RewriteRuleTests.cs @@ -553,6 +553,35 @@ public void MultipleMergeRules() AssertMorphsEqual(morpher.ParseWord("bttbtt"), "27"); } + [Test] + public void MergeWithBoundary() + { + var merge = new RewriteRule + { + Name = "merge", + Lhs = Pattern + .New() + .Annotation(Character(Table3, "b")) + .Annotation(Character(Table3, "+")) + .Annotation(Character(Table3, "b")) + .Value, + }; + merge.Subrules.Add( + new RewriteSubrule + { + Rhs = Pattern + .New() + .Annotation(Character(Table3, "p")) + .Annotation(Character(Table3, "i")) + .Value, + } + ); + Morphophonemic.PhonologicalRules.Add(merge); + + var morpher = new Morpher(TraceManager, Language, 1); + AssertMorphsEqual(morpher.ParseWord("apia"), "39"); + } + [Test] public void ExpandRules() { From cdeb661b1b53ffc3d0b3676c5a565f62b5454b6c Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Mon, 12 Oct 2026 09:10:38 -0700 Subject: [PATCH 3/7] Rename prune to zipper --- .../PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs index 67e23ddf2..0e60e9bf0 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs @@ -42,7 +42,7 @@ private void Unapply(Match targetMatch, Range range, ShapeNode curNode = IsTargetEmpty ? range.Start : range.End; ShapeNode targetNode = range.Start; int targetPos = 0; - bool prune = true; + bool zipper = true; foreach ( Constraint constraint in _analysisRhs.Children.CastToConstraints( "The target (left-hand side) of a rewrite rule" @@ -53,7 +53,7 @@ Constraint constraint in _analysisRhs.Children.CastToConstraint if (varBindings != null) fs.ReplaceVariables(varBindings); if ( - prune + zipper && targetPos < _targetCount && constraint.Type() == HCFeatureSystem.Segment && targetNode.Type() == HCFeatureSystem.Segment @@ -68,7 +68,7 @@ Constraint constraint in _analysisRhs.Children.CastToConstraint { // Add an optional node for fs. curNode = targetMatch.Input.Shape.AddAfter(curNode, fs, true); - prune = false; + zipper = false; } } From 2dd692c7e24bde85900ab0c2d00678a4af38b49b Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Tue, 13 Oct 2026 08:28:46 -0700 Subject: [PATCH 4/7] Fix issue reported by Claude --- .../PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs index 0e60e9bf0..948dd61eb 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs @@ -60,7 +60,7 @@ Constraint constraint in _analysisRhs.Children.CastToConstraint ) { // Union fs into the existing node's feature structure. - targetNode.Annotation.FeatureStruct.Union(fs); + targetNode.Annotation.FeatureStruct.Union(fs, varBindings); targetNode = targetNode.Next; targetPos++; } From c5f67697b715c1cc45055f260d281b3de6dc5cdc Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Wed, 14 Oct 2026 10:47:08 -0700 Subject: [PATCH 5/7] Address F7 --- .../PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs index 948dd61eb..0e60e9bf0 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs @@ -60,7 +60,7 @@ Constraint constraint in _analysisRhs.Children.CastToConstraint ) { // Union fs into the existing node's feature structure. - targetNode.Annotation.FeatureStruct.Union(fs, varBindings); + targetNode.Annotation.FeatureStruct.Union(fs); targetNode = targetNode.Next; targetPos++; } From 874645014712a125b9997b408b0353d25b458b58 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Mon, 19 Oct 2026 07:38:05 -0700 Subject: [PATCH 6/7] Add code to deal with alpha variables --- .../NarrowAnalysisRewriteRuleSpec.cs | 2 ++ .../PhonologicalRules/RewriteRuleTests.cs | 22 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs index 0e60e9bf0..6380eaab1 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/PhonologicalRules/NarrowAnalysisRewriteRuleSpec.cs @@ -57,6 +57,8 @@ Constraint constraint in _analysisRhs.Children.CastToConstraint && targetPos < _targetCount && constraint.Type() == HCFeatureSystem.Segment && targetNode.Type() == HCFeatureSystem.Segment + && !fs.HasVariables // This can be removed when Union is fixed. + && !targetNode.Annotation.FeatureStruct.HasVariables ) { // Union fs into the existing node's feature structure. diff --git a/tests/SIL.Machine.Morphology.HermitCrab.Tests/PhonologicalRules/RewriteRuleTests.cs b/tests/SIL.Machine.Morphology.HermitCrab.Tests/PhonologicalRules/RewriteRuleTests.cs index 7a11afcfa..dbd430a58 100644 --- a/tests/SIL.Machine.Morphology.HermitCrab.Tests/PhonologicalRules/RewriteRuleTests.cs +++ b/tests/SIL.Machine.Morphology.HermitCrab.Tests/PhonologicalRules/RewriteRuleTests.cs @@ -582,6 +582,28 @@ public void MergeWithBoundary() AssertMorphsEqual(morpher.ParseWord("apia"), "39"); } + [Test] + public void AlphaAgreementMergeFindsAbbaFromApa() + { + FeatureStruct alphaVoicedConsonant = FeatureStruct + .New(Language.PhonologicalFeatureSystem) + .Symbol(HCFeatureSystem.Segment) + .Symbol("cons+") + .Feature("vd") + .EqualToVariable("a") + .Value; + var merge = new RewriteRule + { + Name = "alphaMerge", + Lhs = Pattern.New().Annotation(alphaVoicedConsonant).Annotation(alphaVoicedConsonant).Value, + }; + merge.Subrules.Add(new RewriteSubrule { Rhs = Pattern.New().Annotation(Character(Table3, "p")).Value }); + Morphophonemic.PhonologicalRules.Add(merge); + + var morpher = new Morpher(TraceManager, Language); + AssertMorphsEqual(morpher.ParseWord("apa"), "40"); + } + [Test] public void ExpandRules() { From 4294d2e3d978b635f4326a5b450815258ca9e51d Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Mon, 19 Oct 2026 09:14:08 -0700 Subject: [PATCH 7/7] Fix format --- .../PhonologicalRules/RewriteRuleTests.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/SIL.Machine.Morphology.HermitCrab.Tests/PhonologicalRules/RewriteRuleTests.cs b/tests/SIL.Machine.Morphology.HermitCrab.Tests/PhonologicalRules/RewriteRuleTests.cs index dbd430a58..be7135bc3 100644 --- a/tests/SIL.Machine.Morphology.HermitCrab.Tests/PhonologicalRules/RewriteRuleTests.cs +++ b/tests/SIL.Machine.Morphology.HermitCrab.Tests/PhonologicalRules/RewriteRuleTests.cs @@ -595,9 +595,15 @@ public void AlphaAgreementMergeFindsAbbaFromApa() var merge = new RewriteRule { Name = "alphaMerge", - Lhs = Pattern.New().Annotation(alphaVoicedConsonant).Annotation(alphaVoicedConsonant).Value, + Lhs = Pattern + .New() + .Annotation(alphaVoicedConsonant) + .Annotation(alphaVoicedConsonant) + .Value, }; - merge.Subrules.Add(new RewriteSubrule { Rhs = Pattern.New().Annotation(Character(Table3, "p")).Value }); + merge.Subrules.Add( + new RewriteSubrule { Rhs = Pattern.New().Annotation(Character(Table3, "p")).Value } + ); Morphophonemic.PhonologicalRules.Add(merge); var morpher = new Morpher(TraceManager, Language);