Repository navigation
Hi TN: Add support for Range normalization - #513
shrpawar-alt wants to merge 5 commits into
Conversation
Signed-off-by: Shreyas Pawar <shrpawar@nvidia.com>
Signed-off-by: Shreyas Pawar <shrpawar@nvidia.com>
for more information, see https://pre-commit.ci
| major_minor_graphs = [] | ||
| minor_only_graphs = [] | ||
|
|
||
| for major in all_major_names: |
There was a problem hiding this comment.
any solution for not looping all major_minor_currencies?
There was a problem hiding this comment.
I think the loop has to stay: the token has the currency before the number, but the spoken output needs it after, so each currency needs its own branch (and the minor-unit word, पैसे / सेंट / पेंस, depends on the currency too). Having the tagger emit the number first would avoid the loop here, but then the tagger needs a copy of the number graph per currency, which is what made the classify grammar 55 MB (now ~45 MB). Added a comment above the loop for more clarity.
| all_currencies += [maj for _, maj in load_labels(get_abs_path("data/money/currency_singular.tsv"))] | ||
|
|
||
| native_major_only_graphs = [] | ||
| for major in set(all_currencies): |
There was a problem hiding this comment.
any solution for not looping all all_currencies?
There was a problem hiding this comment.
Good catch; that block was redundant. Removed it, as all tests are passing without it.
Signed-off-by: Shreyas Pawar <shrpawar@nvidia.com>
for more information, see https://pre-commit.ci
| # says the currency after the number). Emit two money tokens with "से" between them instead. | ||
| money_separator = pynutil.insert(' } tokens { name: "से" } tokens { ') | ||
| range_money = money + delete_space + pynutil.delete("-") + delete_space + money_separator + money | ||
| range_money = pynutil.add_weight(range_money, 0.1) |
There was a problem hiding this comment.
try to remove/limit the use of custom weight
| range_tagger = RangeFst( | ||
| time=time_final, | ||
| date=date_final, | ||
| cardinal=pynini.compose(cardinal_graph, v_cardinal.fst), |
There was a problem hiding this comment.
might cause error in the future, leave eye on it
folivoramanh
left a comment
folivoramanh
left a comment
There was a problem hiding this comment.
resolve the conflict for merging, too
What does this PR do ?
Problem
each side was normalized on its own (e.g. "दस डॉलर-बीस डॉलर").
("निन्यानबे-एक").
Fix
money values is now read as "X से Y" (10-20 -> "दस से बीस", $10-$20 -> "दस डॉलर से बीस डॉलर").
The serial test case was moved to the range tests.
Other changes made while fixing
because it paired the full measure and money graphs with themselves. To fix this:
standalone measure is unchanged.
(same approach as the English money range).
major_minor_currencies.tsv, and the tagger emits currency_min: "true". The number graph is built
once instead of once per currency. Spoken output is unchanged.
Before your PR is "Ready for review"
Pre checks:
git commit -sto sign.pytestor (if your machine does not have GPU)pytest --cpufrom the root folder (given you marked your test cases accordingly@pytest.mark.run_only_on('CPU')).bash tools/text_processing_deployment/export_grammars.sh --MODE=test ...pytestand Sparrowhawk here.__init__.pyfor every folder and subfolder, includingdatafolder which has .TSV files?Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.to all newly added Python files?Copyright 2015 and onwards Google, Inc.. See an example here.try import: ... except: ...) if not already done.PR Type:
If you haven't finished some of the above items you can still open "Draft" PR.