Repository navigation
HermitCrab: prune copy-agreement matches with an empty copy #645
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -154,16 +154,44 @@ public void WordInitialOptionalNonSegmentIsNeverPruned() | |
| } | ||
|
|
||
| [Test] | ||
| public void EmptyCopyIsNeverPruned() | ||
| public void EmptyCopyAgainstANonEmptyCopyIsPruned() | ||
| { | ||
| (string first, string second) = FindIncompatibleSegments(); | ||
| Pattern<Word, ShapeNode> part = Pattern<Word, ShapeNode>.New("copy").Annotation(AnySegment).ZeroOrMore.Value; | ||
| AffixProcessRule rule = CreateCopyRule(part); | ||
| Word input = CreateInput(first + second); | ||
|
|
||
| // the split with an empty copy has nothing to compare, so only the split of two incompatible copies is removed | ||
| // "whole word + empty" and "empty + whole word" cannot both be copies of one input, and neither can the | ||
| // split of two incompatible halves | ||
| Assert.That(AnalysisForms(false, rule, input), Is.EquivalentTo(new[] { first, first + second })); | ||
| Assert.That(AnalysisForms(true, rule, input), Is.EqualTo(new[] { first + second })); | ||
| Assert.That(AnalysisForms(true, rule, input), Is.Empty); | ||
| } | ||
|
|
||
| [Test] | ||
| public void EmptyCopyIsKeptWhenTheWordHasAnOptionalNode() | ||
| { | ||
| // a restored deletion could fill the empty copy, so the match stays undecidable | ||
| (string first, string second) = FindIncompatibleSegments(); | ||
| Pattern<Word, ShapeNode> part = Pattern<Word, ShapeNode>.New("copy").Annotation(AnySegment).ZeroOrMore.Value; | ||
| AffixProcessRule rule = CreateCopyRule(part); | ||
| Word input = CreateInputWithOptionalSecondSegment(first + second); | ||
|
|
||
| Assert.That(AnalysisForms(true, rule, input), Is.EquivalentTo(AnalysisForms(false, rule, input))); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI: F2. This test would also pass if the rule produced nothing at all, because it only checks that the pruned and unpruned outputs are equal. Pin the unpruned forms first, as |
||
| } | ||
|
|
||
| [Test] | ||
| public void ZeroOrMoreCopyRuleAddsNoSynthesisWorkForAWordItDoesNotReduplicate() | ||
| { | ||
| // sillsdev/machine#634: the match "whole word + empty copy" used to re-run lookup and synthesis | ||
| string[] words = { "pit", "pʰit", "pʰut", "sagd", "sagdv" }; | ||
| (int lookups, int syntheses) without = CountSearchWork(words); | ||
|
|
||
| Morphophonemic.MorphologicalRules.Add( | ||
| CreateCopyRule(Pattern<Word, ShapeNode>.New("copy").Annotation(AnySegment).ZeroOrMore.Value) | ||
| ); | ||
|
|
||
| Assert.That(without.syntheses, Is.GreaterThan(0)); | ||
| Assert.That(CountSearchWork(words), Is.EqualTo(without)); | ||
| } | ||
|
|
||
| [TestCase(false, 3)] | ||
|
|
@@ -550,6 +578,32 @@ private RewriteRule CreateBetweenVowelsRewrite(CharacterDefinitionTable table, s | |
| return rule; | ||
| } | ||
|
|
||
| private (int Lookups, int Syntheses) CountSearchWork(string[] words) | ||
| { | ||
| var morpher = new Morpher(new TraceManager { IsTracing = true, RetainWords = false }, Language, 1); | ||
| int lookups = 0; | ||
| int syntheses = 0; | ||
| foreach (string word in words) | ||
| { | ||
| morpher.ParseWord(word, out object trace); | ||
| foreach (Trace node in Flatten((Trace)trace)) | ||
| { | ||
| if (node.Type == TraceType.LexicalLookup) | ||
| lookups++; | ||
| else if (node.Type == TraceType.WordSynthesis) | ||
| syntheses++; | ||
| } | ||
| } | ||
| return (lookups, syntheses); | ||
| } | ||
|
|
||
| private static IEnumerable<Trace> Flatten(Trace trace) | ||
| { | ||
| yield return trace; | ||
| foreach (Trace descendant in trace.Children.SelectMany(Flatten)) | ||
| yield return descendant; | ||
| } | ||
|
|
||
| private Morpher CreateMorpher(bool pruneCopies, int maxDegreeOfParallelism = 0) | ||
| { | ||
| return new Morpher(TraceManager, Language, maxDegreeOfParallelism) { PruneDisagreeingCopies = pruneCopies }; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI: F1. The
HasDisagreeingCopiessummary (lines 57-64) still lists only "a part the rule modifies, or a copy with an optional node that is not a segment" as never reported. This branch adds a third case: an empty copy in a word that has any optional node. Add it to that sentence so the documented contract matches the code.