fix(en-tn): verbalize plural time unit abbreviations (mins/hrs/secs/kgs/wks/mos/yrs) - #509
Open
JMak-Security wants to merge 3 commits into
Open
JMak-Security wants to merge 3 commits into
JMak-Security wants to merge 3 commits into
Conversation
en/data/measure/unit.tsv maps the singular abbreviations min, sec, hr to their spoken forms, but not the plural written forms mins, hrs, secs. Since measure normalization only fires when the written token is recognized as a unit, the plural forms pass through unverbalized: '5 mins' -> 'five mins' (should be 'five minutes') '2-3 mins' -> 'two - three mins' (should be 'two to three minutes') The range case is the more visible failure: the measure range rule can't fire either, so the hyphen from '2-3' is left in the output -- for TTS this renders as silence. No grammar change is needed. MeasureFst already derives the spoken plural from the singular value via graph_unit_plural = convert_space(graph_unit @ SINGULAR_TO_PLURAL) (taggers/measure.py), so a plural written key only needs a row mapping to the existing singular spoken form -- the same pattern already used for 'lbs -> pound'. Fixes NVIDIA#477 Signed-off-by: Jason Mak <squrrielbro@gmail.com>
Follow-up per review feedback: the same missing-plural-written-form bug affects kg/kgs, wk/wks, mo/mos, and yr/yrs, not just the min/sec/hr family fixed in the previous commit. Scoped to these four specifically, not a mechanical sweep of every abbreviation in the file: kgs mirrors the already-existing lbs -> pound precedent (mass units commonly get colloquial 's' plurals despite SI style guides discouraging it), and wks/mos/yrs are the same casual time-count family as mins/hrs/secs. Left the SI symbol portion of the file (km, mm, GB, kW, etc.) untouched, since those are not commonly written with a colloquial 's' plural the way day/week/month/year/ hour/minute/second/pound/kilogram are. Signed-off-by: Jason Mak <squrrielbro@gmail.com>
Covers mins/hrs/secs/kgs/wks/mos/yrs plus singular controls, so the plural verbalization added in this PR is locked in and any future regression in the measure tagger surfaces immediately. Verified: pytest tests/nemo_text_processing/en/test_measure.py --cpu reports 268 passed, 0 failed (253 pre-existing + 15 new). Signed-off-by: JMAK-Security <squrrielbro@gmail.com>
JMak-Security
force-pushed
the
fix/en-tn-plural-time-unit-abbreviations
branch
from
October 5, 2026 03:38
39ddfdd to
2f5d3ec
Compare
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.
Supersedes #479, which the stale bot closed while CI was green and the PR was mergeable.
GitHub will not let me reopen #479: rebasing the branch onto current
mainrequired a force-push, and reopening is refused after a force-push (state cannot be changed ... the branch was force-pushed or recreated). So this is the same fix, rebased, with the regression coverage that was missing from the original revision, and a fresh PR to go with it.What this does
en/data/measure/unit.tsvmaps the singular time abbreviationsmin,sec,hrto their spoken forms, but not the plural written formsmins,hrs,secs. Since measure normalization only fires when the written token is recognised as a unit, the plural forms pass through unverbalized:The range case is the more visible failure: the measure range rule cannot fire either, so the hyphen from
2-3is left in the output, which TTS renders as silence/a pause.Scope
Limited to the units that take a colloquial
splural:min/mins,sec/secs,hr/hrs,kg/kgs,wk/wks,mo/mos,yr/yrs. SI symbols (km,mm,GB,kW, …) are deliberately untouched, as those are not written that way.kgsmirrors the already-existinglbs -> poundprecedent.The scope question raised in review on #479 is resolved in favour of exactly this set.
Verification
Run with python 3.10 +
pynini==2.1.6.post1on both trees:main(before)5 minsfive minsfive minutes2 hrstwo HRStwo hours30 secsthirty secsthirty seconds3 kgsthree kgsthree kilograms2 wkstwo wkstwo weeks6 mossix mossix months5 yrsfive yrsfive years2-3 minstwo - three minstwo to three minutes5 minfive minutesfive minutes(unchanged)1 secone secondone second(unchanged)main: 253 passed15 cases were added to
en/data_text_normalization/test_cases_measure.txtcovering all seven plural units plus singular controls, so this cannot silently regress. The original revision changed data only, with no test exercising the behaviour, which is likely why it kept getting passed over.Changes
fix(en-tn): verbalize plural time unit abbreviations (mins/hrs/secs)fix(en-tn): add kgs/wks/mos/yrs plural unit abbreviationstest(en-tn): add regression cases for plural unit abbreviationsCould someone take a look when convenient? And if
enTN fixes are no longer landing in this repo, I would appreciate a pointer to where they should go instead — happy to re-target.