Export LabelLookAheadRelabeler - #314
Open
npuichigo wants to merge 1 commit into
Open
Conversation
`LabelLookAheadRelabeler::{init, relabel}` are declared `pub`, but the
re-export in `lookahead_matchers/mod.rs` is `pub(super)`, so the type is
unreachable from outside the crate.
This makes it impossible to relabel an FST against an already-built
lookahead FST. The only external entry point left is
`MatcherFst::new_with_relabeling`, which builds the reachability tables
and relabels the other FST in a single call, so the tables have to be
rebuilt on every composition.
That is prohibitive whenever one FST is static and the other varies: in a
text-normalization workload, building the tables for a 187k-state grammar
takes ~300 ms while each input is a short linear acceptor. With the
relabeler exported, the tables are built once at startup and each input is
relabeled into the grammar's index space before composing, which measured
14.4 ms -> 3.8 ms per input with byte-identical output.
Widening the re-export to `pub` matches the other items in the module and
adds no new API surface beyond the already-`pub` type.
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
1 participant
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.
What
Widen one re-export from
pub(super)topub.Why
LabelLookAheadRelabelerhaspub fn initandpub fn relabel, butlookahead_matchers/mod.rsre-exports the type aspub(super), so neither iscallable from outside the crate.
That blocks the natural way to use lookahead composition when one side is static
and the other changes per call:
LabelReachableData) once, thenStep 2 needs
LabelLookAheadRelabeler::relabel. Without it the only reachableentry point is
MatcherFst::new_with_relabeling, which fuses "build thereachability tables" with "relabel the other FST", so the tables are rebuilt on
every composition.
Impact
Measured on a text-normalization workload: short linear input acceptors composed
against a static 187k-state grammar, then shortest path.
compose)Building the tables moves to startup (~300 ms, one-off). Output is unchanged:
verified byte-for-byte against plain
composeacross a 184-case regressioncorpus.
For reference, OpenFST (via pynini) on the same grammar and corpus measures
7.4 ms per input, so this path makes rustfst roughly 2x faster than OpenFST here
— but only if the relabeler is reachable.
Risk
Should be none: the type and both of its methods are already
pub; only themodule re-export is widened, matching the other items in that module.
Note
Assembling lookahead composition currently requires hand-writing the nested
matcher/filter generics, since
ComposeFilterEnumhas no lookahead variant andcompose_static::create_matcheralways builds aSortedMatcher. Happy tofollow up with a convenience entry point and/or docs if that would be welcome.