fix(en-tn): verbalize plural time unit abbreviations (mins/hrs/secs) - #479
fix(en-tn): verbalize plural time unit abbreviations (mins/hrs/secs)#479JMak-Security wants to merge 2 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>
|
This fix works for mins/hrs/secs, but there are many similar cases — plural abbreviations of units already in unit.tsv, e.g.: '10 kgs' -> 'ten kgs' (kg is a key, kgs is not) Is there a reason plural written keys are generally excluded ? |
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>
|
@dk-dineshkannaa, thank you, I've confirmed all four against |
Summary
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 recognized as a unit, the plural forms pass through unverbalized: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/a pause.Root cause
Confirmed directly against
unit.tsvat HEAD (acacc21b):lbs -> pound(L45) andsec/min/hr(L128-130) exist, but no plural written forms for the time units.Fix
No grammar change is needed.
MeasureFstalready derives the spoken plural from the singular value:So a plural written key only needs a row mapping to the existing singular spoken form -- the same pattern already used for
lbs -> pound. This adds:grouped with their singular counterparts (
sec/min/hr) in the file.This PR is scoped to the confirmed bug only. The issue also proposes a separate extensibility mechanism (an
extra_unit_filehook to let callers append custom units without regenerating the shipped FAR cache) -- that's a larger design question spanningNormalizerconstruction,MeasureFst, and FAR cache invalidation, and is left for separate discussion/PR.Testing
I couldn't get a local
pynini/nemo-text-processinginstall running in my environment (cdifflib, a transitive dependency, needs a C++ build toolchain I don't have here), so I verified this the way available to me:unit.tsvat the exact commit HEAD currently points to lacksmins/hrs/secs, matching the bug report precisely.taggers/measure.pydirectly and traced thegraph_unit_plural = convert_space(graph_unit @ SINGULAR_TO_PLURAL)composition myself to independently confirm the plural-derivation mechanism the fix relies on, rather than taking the issue's claim at face value.lbs -> poundrow.nemo-text-processing==1.2.0/pynini 2.1.6.post1, with before/after output for'5 mins','2-3 mins','2-3 hrs','30-45 secs', plus regression checks ('1 min'unaffected,'2013-2016'unaffected).Fixes #477