Skip to content

Keep "point" when it does not start a decimal in the English normalizer - #2889

Open
MohammadHijjawi97 wants to merge 1 commit into
openai:mainfrom
MohammadHijjawi97:fix-normalizer-dropped-point
Open

MohammadHijjawi97 wants to merge 1 commit into
openai:mainfrom
MohammadHijjawi97:fix-normalizer-dropped-point

Conversation

@MohammadHijjawi97

Copy link
Copy Markdown

EnglishNumberNormalizer.process_words handles point in two cases: if the next word is not a number word, point is emitted as-is; if the next word is a decimal digit (or numeric), a . is appended. When the next word is a number word but not a decimal digit (and, minus, first, dollars, ...) neither case applies and the word is silently dropped:

input before after
that was the point and then we left that was the and then we left that was the point and then we left
match point minus two match -2 match point -2
the point first the 1st the point 1st
good point dollars good dollars good point dollars
two point and 2 and 2 point and

The fix flushes any pending value and emits point, the same way the double/triple branch already handles a non-numeric follower. Decimal cases are unchanged (3 point 14, one point oh five, two point two five dollars still give 3.14, 1.05, $2.25).

Added four asserts to the parametrized test_number_normalizer (they fail without the change); tests/test_normalizer.py, test_tokenizer.py and the non-CUDA test_timing.py tests pass, and black/isort/flake8 at the pre-commit pins are clean.

EnglishNumberNormalizer only emitted "point" when the next word was not a
number word at all. When the next word was number-related but not a
decimal digit ("and", "minus", "first", "dollars", ...), the branch did
nothing and the word was silently dropped:

    "that was the point and then"  -> "that was the and then"
    "match point minus two"        -> "match -2"

Emit "point" (after flushing any pending value) in that case, as the
"double"/"triple" branch already does.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant