De itn decimal decoupled v2 - #497
Open
adelina23dunina wants to merge 6 commits into
Open
adelina23dunina wants to merge 6 commits into
adelina23dunina wants to merge 6 commits into
Conversation
Signed-off-by: Adelina Dunina <adunina@nvidia.com>
Rewrite the German ITN decimal grammar so it no longer wraps the TN decimal tagger and verbalizer, mirroring what was already done for the cardinal tagger. - Merge data/cardinal and data/decimal into a shared data/numbers directory, so digits.tsv, zero.tsv and quantity.tsv exist once. - Expose magnitude, magnitude_words and scale_forms on CardinalFst and let the decimal tagger reuse that graph instead of building its own. - Add get_quantity: a bare 1-999 integer or a decimal followed by a magnitude word gets a quantity field, while hundert and tausend after a bare integer stay with the cardinal grammar. - Denormalise tausend, million and milliarde to Tsd., Mio. and Mrd.; larger magnitudes keep the full word. Add quantity_nondeterministic.tsv for the deterministic=False variant and thread the deterministic flag through ClassifyFst, including the FAR cache file name. - Cover the spoken half and quarter forms: einhalb, anderthalb, einanderthalb, einviertel and dreiviertel. - Rewrite the decimal verbalizer to read the negative, integer_part, fractional_part and quantity fields directly. - Grow test_cases_decimal.txt from 9 to 36 cases. Signed-off-by: Adelina Dunina <adunina@nvidia.com>
mgrafu
reviewed
Sep 18, 2026
| # ordered by value, each scale is three decimal digits larger than the previous one | ||
| MAGNITUDE_SCALES = ("hundert", "tausend", "million", "milliarde", "billion", "billiarde", "trillion", "trilliarde") | ||
| # after a bare integer these stay with the cardinal grammar: zwei tausend -> 2.000 | ||
| CARDINAL_SCALES = ("hundert", "tausend") |
Collaborator
There was a problem hiding this comment.
data file instead of hardcoding. it also looks like you could just use it directly in decimals
mgrafu
reviewed
Sep 18, 2026
| @@ -1,4 +1,4 @@ | |||
| # Copyright (c) 2021, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | |||
| # Copyright (c) 2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved. | |||
Collaborator
There was a problem hiding this comment.
I'm still seeing changes in 4 files. please check
mgrafu
reviewed
Sep 18, 2026
| from nemo_text_processing.text_normalization.normalize_with_audio import NormalizerWithAudio | ||
|
|
||
| from ..utils import CACHE_DIR, RUN_AUDIO_BASED_TESTS, parse_test_case_file | ||
| from tests.nemo_text_processing.utils import CACHE_DIR, RUN_AUDIO_BASED_TESTS, parse_test_case_file |
Collaborator
There was a problem hiding this comment.
does this need the change?
Collaborator
There was a problem hiding this comment.
still seeing changes in this file
mgrafu
reviewed
Sep 18, 2026
|
|
||
| class TestDecimal: | ||
| inverse_normalizer = InverseNormalizer(lang='de', cache_dir=CACHE_DIR, overwrite_cache=False) | ||
| inverse_normalizer = InverseNormalizer(lang="de", cache_dir=CACHE_DIR, overwrite_cache=False) |
Collaborator
There was a problem hiding this comment.
does this need the change?
mgrafu
reviewed
Sep 18, 2026
| @parameterized.expand(parse_test_case_file('de/data_inverse_text_normalization/test_cases_decimal.txt')) | ||
| @pytest.mark.run_only_on('CPU') | ||
| @parameterized.expand(parse_test_case_file("de/data_inverse_text_normalization/test_cases_decimal.txt")) | ||
| @pytest.mark.run_only_on("CPU") |
Collaborator
There was a problem hiding this comment.
does this need the change?
Use the relative 'from ..utils import' in the decimal test, matching the convention of the other test files, and restore the original quoting along with it. Bump the copyright year to 2026 on the de ITN files this branch rewrites. Signed-off-by: Adelina Dunina <adunina@nvidia.com>
adelina23dunina
force-pushed
the
de_itn_decimal_decoupled_v2
branch
from
September 21, 2026 15:32
1f11290 to
9e02b4f
Compare
for more information, see https://pre-commit.ci
Millionen, Milliarden and tausend are denormalised to their written form instead of Mio., Mrd. and Tsd. The abbreviations become a deterministic=False variant only, and the decimal tagger stops importing from the cardinal tagger, which finishes the decoupling started in the previous commit. - data/numbers/quantity.tsv: map tausend, million, millionen, milliarde and milliarden to their full forms. The noun magnitudes are capitalised, the numerals hundert and tausend stay lower case. - data/numbers/quantity_nondeterministic.tsv: delete. The five abbreviated forms are now a string_map inside get_quantity, next to the branch that uses them, and they no longer carry an extra 0.001 weight. - taggers/cardinal.py: drop the MAGNITUDE_SCALES and CARDINAL_SCALES module constants. quantity.tsv is ordered by value and a scale name prefixes all of its own forms and no other scale, so the scale groups are derived from the file itself. scale_forms and magnitude_words become locals and big_magnitude_words is exposed in their place, ready for the decimal tagger to consume. - taggers/decimal.py: drop the CARDINAL_SCALES import and build big_quantity from cardinal.big_magnitude_words instead of subtracting hundert and tausend locally. Update the class and get_quantity docstrings to the full forms. - verbalizers/decimal.py: update the docstring example to Millionen. - tests/de/data_inverse_text_normalization/test_cases_decimal.txt: switch the expected values to the full forms and add minus zehn millionen, covering a negative sign in front of a bare integer with a quantity. - tests/de/test_decimal.py: re-enable the normalization test that was left wrapped in a triple-quoted string and restore the single-line imports. - Restore the copyright years the branch had bumped to 2026. Signed-off-by: Adelina Dunina <adunina@nvidia.com>
The five spoken fraction words and the three abbreviated magnitudes were written out as pynini.cross and pynini.string_map calls inside the tagger. Move both into TSV files, so the decimal grammar carries no number tables of its own, the way digits.tsv, zero.tsv and quantity.tsv are already handled. - data/numbers/fractions.tsv: add einhalb, anderthalb, einanderthalb, einviertel and dreiviertel with their decimal values. The file stores one value per word, e.g. dreiviertel -> 0,75, and the tagger derives the integer_part and fractional_part fields from that value. - data/numbers/quantity_nondeterministic.tsv: restore the file the previous commit inlined. The five abbreviated forms read better next to quantity.tsv than as a string_map buried in get_quantity. - taggers/decimal.py: replace the three hand-written fraction graphs with value_to_fields and value_to_fractional, which rewrite a value from fractions.tsv into tagger fields. "einhalb" still attaches to a preceding integer, so it contributes only the digits after the comma, while the other four carry their own integer part and stand on their own. Read the abbreviations with string_file again. - taggers/decimal.py: drop the comments that restated the code, namely the two on graph_integer_or_zero and graph_string_of_digits and the three "Coverage for verbalized ..." headings, one of which misspelled anderthalb. Keep a single comment where the reason is not visible from the code: why einhalb feeds only the fractional part while the other four do not. Rewrite the class docstring to show the tagged output instead of the denormalised string, and to state that magnitude nouns are capitalised while the numeral tausend is not. - tests/de/data_inverse_text_normalization/test_cases_decimal.txt: add zwei anderthalb, pinning that a standalone fraction does not attach to a preceding integer, plus minus einviertel and minus zweieinhalb to cover the sign in front of both fraction shapes. Signed-off-by: Adelina Dunina <adunina@nvidia.com>
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.
What does this PR do ?
Decouples the German ITN decimal grammar from the TN decimal tagger and verbalizer, so ITN builds its own decimal tagger, verbalizer and shared number data, the same way the cardinal grammar was decoupled in #487.
Details
data/cardinalanddata/decimalare merged into a shareddata/numbersdirectory, so
digits.tsv,zero.tsvandquantity.tsvexist once.CardinalFstexposesmagnitude,magnitude_wordsandscale_forms; thedecimal tagger reuses that graph instead of building its own.
get_quantitytags a bare 1-999 integer or a decimal followed by a magnitudeword with a
quantityfield.hundertandtausendafter a bare integer staywith the cardinal grammar, so
zwei tausendis still2.000.tausend,millionandmilliardeare denormalised toTsd.,Mio.andMrd.; larger magnitudes keep the full word.quantity_nondeterministic.tsvcovers the
deterministic=Falsevariant.einhalb,anderthalb,einanderthalb,einviertel,dreiviertel.test_cases_decimal.txtgrows from 9 to 36 cases.Open question for reviewers
The abbreviations currently need a deterministic / non-deterministic split that no
other language has. Native speakers tell me nothing above Milliarde is ever
abbreviated, and even Tsd./Mio./Mrd. alternate freely with the full words. I am
happy to drop the abbreviations entirely and keep this in line with the other
languages if that is preferable — would like a second opinion before refactoring.
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: