fix(itn/en): read spoken fractions instead of misfiring the ordinal tagger (#82) - #85
Merged
Conversation
…agger (#82) `normalize_sentence("use one third of a cup")` returned "use 4th of a cup": English ITN had no fraction tagger, so "one third" fell to the ordinal tagger, whose compound-ordinal path blindly added prefix + ordinal (1 + 3 = "4th"). - Add `itn::en::fraction`, run before ordinal in both the single-expression and sentence pipelines (priority 78, below decimal/above ordinal). It emits N/D for spoken fractions: "one third" -> 1/3, "two thirds" -> 2/3, "three quarters" -> 3/4, "twenty two thirds" -> 22/3. - Disambiguate via denominator number: a plural denominator ("thirds") is always a fraction; a singular one ("third") only with numerator one, so "twenty third" stays the ordinal 23rd. Exclude "first"/"second" ("one second" is a duration) and keep singular scale words as ordinals ("one hundredth" -> 100th). The article "a" is not a numerator, so "a quarter of the pizza" is left unchanged. - Guard the ordinal compound-addition path: the prefix must be a round multiple of the next power of ten, so it no longer fabricates ordinals from "one" + ones-ordinal ("one second" no longer becomes "3rd"). Adds unit tests for the fraction tagger and issue #82 integration tests.
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.
Fixes #82.
Problem
nemo_normalize_sentence("use one third of a cup")returneduse 4th of a cup.English ITN had no fraction tagger (fr/de/es/hi/ja/zh do), so
one thirdfell through to the ordinal tagger. Its compound-ordinal path blindly addsprefix + ordinal— correct fortwenty first(20 + 1 = 21st) but nonsense forone third(1 + 3 = 4th). The bug hits every ones-cardinal + ones-ordinal denominator (one fifth→ 6th,one eighth→ 9th, ...); other phrasings leaked the numerator instead (two thirds→2 thirds,one half→1 half).Fix
itn::en::fractiontagger, run before ordinal in both the single-expression and sentence pipelines (priority 78, belowdecimal/aboveordinal). EmitsN/D:one third→1/3,one half→1/2,one quarter→1/4two thirds→2/3,three quarters→3/4,twenty two thirds→22/3thirds) is always a fraction; a singular one (third) is a fraction only with numerator one. Sotwenty thirdstays the ordinal 23rd, whileone thirdbecomes 1/3.first/second—one secondis a duration, not1/2.one hundredth→ 100th (onlytwo hundredths→ 2/100).ais not a numerator, soa quarter of the pizzais left unchanged (matches existing behavior/test).one+ ones-ordinal (one secondno longer becomes3rd). Genuine compounds (twenty first,one hundred tenth) are unaffected.Tests
fraction.rs(singular/plural, deferral to ordinals, excluded denominators, article-is-not-a-numerator).en_tests.rsfor the reported case + ordinal regression guards.fst-enginefeature still builds;cargo fmt --checkclean.Scope
Default (hand-written Rust) taggers only — the
fst-enginepath is untouched.