Repository navigation
Stop synthesis trying affix allomorphs after the one analysis recorded - #560
Open
johnml1135 wants to merge 2 commits into
Open
johnml1135 wants to merge 2 commits into
johnml1135 wants to merge 2 commits into
Conversation
Analysis records which allomorph of each affix rule it unapplied, and synthesis only keeps output from that allomorph or an earlier one. Later allomorphs were still pattern-matched and built, then discarded. The loop in SynthesisAffixProcessRule.Apply now ends at the recorded allomorph. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #560 +/- ##
==========================================
+ Coverage 74.57% 74.78% +0.21%
==========================================
Files 453 453
Lines 38342 38345 +3
Branches 5284 5286 +2
==========================================
+ Hits 28593 28676 +83
+ Misses 8583 8511 -72
+ Partials 1166 1158 -8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Moves the cutoff inside the branch that already decides applicability, so it and IsAllomorphApplicable cannot drift apart. Output is unchanged. Suggested in review. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Public API, target framework, package, machine.py parity: None verified. Findings: F1 addressed. Reviewed at aabf752 |
jtmaxwell3
approved these changes
Oct 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Start here:
src/SIL.Machine.Morphology.HermitCrab/MorphologicalRules/SynthesisAffixProcessRule.cs-- the six added lines inApplythat end the allomorph loop at the recorded allomorph.Status: open; CI green on this head (the
claudejob is skipped).Synthesis stops trying an affix rule's allomorphs once it passes the one analysis recorded (
Word.CurrentRecordedAllomorph, new and internal). Parses are identical on every corpus measured; only work whose output was always discarded is removed. No public API changes. On real grammars the saving in total work is small (0.007% on Aweti to 0.35% on Mbugwe); it is largest where an affix has a long list of environment-conditioned allomorphs (8.45% and 9.49% on Turkic- and Bantu-style stress fixtures).The unknown a reviewer arrives with is whether skipping allomorphs can ever drop an output. It cannot: an output from an allomorph after the recorded one fails
IsAllomorphApplicableand was never added (argument in the first accordion).Where to look:
AffixProcessRuleTestspin the cutoff and the earlier-allomorph cases:SynthesisStopsAtRecordedAllomorph,SynthesisTriesEveryAllomorphUpToRecordedLast,EarlierEnvironmentFreeAllomorphStillBlocksLaterOnes.NotApplied/pattern entries for allomorphs after the recorded one, since they are no longer tried.Not here: a public on/off switch, the research harness and stress fixtures (see #490).
Verification on this branch:
dotnet test tests/SIL.Machine.Morphology.HermitCrab.Tests -c Releasepassed with 0 failures (and none onorigin/master);dotnet csharpier check .andpwsh scripts/comment-hygiene.ps1clean. Not run:./local_check.sh --agent-strictend to end.Next: approve, or tell me which corpus you want measured again.
Reading this a year from now -- start here
The change is a loop cutoff in
SynthesisAffixProcessRule.Apply. The research behind it is experiment E1 of #490 (open); the harness and stress fixtures stayed there, so the measurements below cannot be reproduced from this branch alone.Soundness argument
An output from allomorph
i > rfailsIsAllomorphApplicableand was never added.appliedAllomorphIndicesis copied into each output when it is built (UnionWithinWord.MorphologicalRuleApplied), so indices added afterrnever reachr's output. Allomorphsi <= rrun exactly as before, so the disjunctivebreakand the recorded indices are unchanged. With no recorded allomorph (null, or not in the list),recordedIndexstays -1 and the loop is untouched. Compounding and realizational rules do not use recorded allomorphs and are left alone. The new tests also check that the last allomorph recorded still applies and that an earlier environment-free allomorph still blocks later ones. With thebreakdisabled, two of the three fail.Evidence: measured effect in steps
Synthesis steps with the cutoff off and on, same words in both arms. "Total work" is analysis events plus synthesis steps. Analysis events and candidate counts were equal in both arms on every corpus, and the parse output was identical per word.
Synthesis is a small share of total work on real grammars (1.0% on Aweti, 9.0% on Mbugwe), so the real-grammar saving in total work is small. An environment-free allomorph already ends the loop; environment-bearing ones never did, so every one was applied. Each rule application cost
Aallomorph applications and now costsr + 1.Cost not in the step counters: one
IndexOfover the rule's allomorphs per rule application, at mostAreference comparisons.Surprising findings: failures seen on the research branch
The HermitCrab test project has no failures on this branch or on
origin/master. Failures seen in the research branch's runs came from the research branch, not this change:truncate-morphotactic*conformance fixtures (nondeterministic, fail without the cutoff) and a producibility-ledger test in the conformance suite (#480, open) that lackedAllomorphNotApplicableafter #480 was merged with #548 (merged). Neither exists on master.🤖 Generated with Claude Code
This change is