Repository navigation
perf(hermitcrab): run the required-slot look-ahead on the shared trailing-slot template walk - #577
Draft
johnml1135 wants to merge 8 commits into
Draft
johnml1135 wants to merge 8 commits into
johnml1135 wants to merge 8 commits into
Conversation
johnml1135
added a commit
that referenced
this pull request
Oct 28, 2026
Rows 43 and 44 move from in progress to done. R1 on #546 is draft PR #577 (commit bbb81a0, shadow 0 violations); R3 ordered look-ahead is draft PR #576 (shadow 0 violations). Adds the round-doc section for both and the PanGloss port issues (sillsdev/PanGloss #65, #66). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #577 +/- ##
==========================================
+ Coverage 74.57% 74.81% +0.24%
==========================================
Files 453 455 +2
Lines 38342 38752 +410
Branches 5284 5376 +92
==========================================
+ Hits 28593 28993 +400
- Misses 8583 8593 +10
Partials 1166 1166 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
johnml1135
added a commit
that referenced
this pull request
Oct 28, 2026
Adds the round-doc section "Combined stack and R2's cost": the full stack against master (work, unification, matcher per grammar), additivity (near-additive on Mbugwe, overlapping on Sena, super-additive on Aweti), R2's root cause (its own probe, low prune rate) and the witness-reuse fix folded into #572 @ 01bdf5a, the 1-4 point residual, and the landing decision: #572 is a draft and lands only on top of #577 and #574. Stress grammars are PR #578 (opt-in, [Explicit], not run in CI). Ledger: row 40 (R2) Outcome records the fold and the landing decision; new row 47 records the combined stack measurement. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…k guard, avoid per-node closures The shared walk is only taken when MaxAlternatives is 0, so its limit check could never fire; the guard comment now names all five fallback conditions; Walk checks admitted templates with a loop instead of a capturing lambda. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#522 removed AnalysisStateKey along with the analysis-cascade memo and kept the same identity as AnalysisStratumRule.AnalysisMergeKey, so the shared-walk equivalence test now compares outputs under that key. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…erial An analysis template partial now stops as soon as a required slot can no longer find the segments its affix inserts, instead of paying for every optional slot before the required one. The test is used only while every slot walked since is a pure copy-and-insert rule, so parses do not change. A count-only shadow mode and a null-by-default check observer let a harness verify soundness and cost on real grammars. Closes #561 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… walk Each template under a trie node gets its own look-ahead and a dead template stops collecting below it while its siblings go on. Closes #566 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
johnml1135
force-pushed
the
perf/hc-lookahead-on-546
branch
from
October 29, 2026 11:10
bbb81a0 to
fcb65d7
Compare
Collaborator
Author
… pure optional slot, and the exhaustive test compares against look-ahead Off. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Contributor
Public API, target framework, package, and Findings: F1 addressed; F2 addressed. Reviewed at c8df81a |
This branch has not been deployed
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/AnalysisAffixTemplatesRule.cs, theWalkmethod -- the per-template look-ahead mask on the shared trie walk. Review only the top commit (fcb65d7); the branch also carries #546 and #575.Status: draft, stacked on #546 (shared trailing-slot trie, open) and #575 (required-slot look-ahead, open). Its base is master, so the PR diff also shows both. CI green. Closes #566.
What it does. The shared trailing-slot walk from #546 now runs #575's required-slot look-ahead at every trie node, for each admitted template below it. A template whose required slot cannot find its material is dead for that subtree while the templates sharing the node carry on; a child whose templates are all dead is skipped before its slot rule runs. On the benchmark grammars, adding the look-ahead to the shared walk cut a further 8.26% (Mbugwe), 25.97% (Sena) and 18.75% (Aweti) of work.
The unknown you arrive with: is it sound with sharing? Dead templates cannot complete (the #575 argument, applied per template), so outputs equal the shared walk without the look-ahead. The mask is one
boolper template per call: no cache, noInterlocked. The sameRequiredSlotLookaheadOff/Shadow/On switch as #575 applies; shadow reports fired, violated and completed-under-fired counts per template.Where to look
LookaheadPrunesOnlyTheTemplateWhoseRequiredSlotIsGone.LookaheadKeepsEveryAttemptWhenBothTemplatesAreFeasible.ShadowLookaheadOnTheSharedWalkNeverViolatesOrChangesTheOutput.LookaheadSharedWalkMatchesPerTemplateWalkOnEveryShortInput.Not here. The shared walk still runs only at
Morpher.MaxDegreeOfParallelism == 1, as in #546; default callers get throughput from #582 (open). The look-ahead adds unification calls: 1.07% (Mbugwe) and 1.40% (Sena) more than #546 alone. Later look-ahead refinements are #576, #579 and #580.Stack and verification. Merge order: #546, #575, then this. Ran earlier: HermitCrab test project, parity against master, #546, #575 and the combination on every benchmark word; shadow-mode violations 0. Not rerun after the rebase onto #522: the measurements below. The four truncate-morphotactic conformance failures are known.
Next: land #546 and #575, then review the top commit.
Reading this a year from now -- start here
This is the join of two independent optimisations: #546 removes repeated slot batches across templates, #575 prunes a partial parse as soon as a required slot's material is gone. Each alone leaves work the other removes, so the join is measured on its own (#566).
Evidence
Analysis non-template events plus synthesis steps, and unification calls including the look-ahead's own checks, master versus each variant. Measured before #522 removed the analysis memo; not rerun.
Added to #546: 8.26%, 25.97% and 18.75% less work on Mbugwe, Sena and Aweti, with 1.07% and 1.40% MORE unification calls on Mbugwe and Sena and 11.18% less on Aweti. Of the shared-slot work (about 65% of events before), 5.0% (Mbugwe), 20.2% (Sena) and 11.4% (Aweti) of events remain after both.
Shadow: 0 violations and 0 completions under a fired check on Mbugwe, Sena, Aweti, Amharic, Indonesian, conformance and the stress fixtures. Parses equal master, #546, #575 and the combination on every word.
Decisions and why
🤖 Generated with Claude Code
This change is