Skip to content

Telugu TN: Cardinal Semiotic Class - #447

Open
nharidas wants to merge 10 commits into
NVIDIA:staging/telugu_tn_v1from
nharidas:feat-cardinal-te
Open

Telugu TN: Cardinal Semiotic Class#447
nharidas wants to merge 10 commits into
NVIDIA:staging/telugu_tn_v1from
nharidas:feat-cardinal-te

Conversation

@nharidas

@nharidas nharidas commented Jul 7, 2026

Copy link
Copy Markdown

What does this PR do ?

Add a one line overview of what this PR aims to accomplish.

Before your PR is "Ready for review"

Pre checks:

  • Have you signed your commits? Use git commit -s to sign.
  • Do all unittests finish successfully before sending PR?
    1. pytest or (if your machine does not have GPU) pytest --cpu from the root folder (given you marked your test cases accordingly @pytest.mark.run_only_on('CPU')).
    2. Sparrowhawk tests bash tools/text_processing_deployment/export_grammars.sh --MODE=test ...
  • If you are adding a new feature: Have you added test cases for both pytest and Sparrowhawk here.
  • Have you added __init__.py for every folder and subfolder, including data folder which has .TSV files?
  • Have you followed codeQL results and removed unused variables and imports (report is at the bottom of the PR in github review box) ?
  • Have you added the correct license header Copyright (c) 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. to all newly added Python files?
  • If you copied nemo_text_processing/text_normalization/en/graph_utils.py your header's second line should be Copyright 2015 and onwards Google, Inc.. See an example here.
  • Remove import guards (try import: ... except: ...) if not already done.
  • If you added a new language or a new feature please update the NeMo documentation (lives in different repo).
  • Have you added your language support to tools/text_processing_deployment/pynini_export.py.

PR Type:

  • New Feature
  • Bugfix
  • Documentation
  • Test

If you haven't finished some of the above items you can still open "Draft" PR.

@nharidas
nharidas force-pushed the feat-cardinal-te branch from 4a71ee5 to f918e66 Compare July 7, 2026 07:57
@nharidas
nharidas marked this pull request as ready for review July 7, 2026 10:52
@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 14 days with no activity. Remove stale label or comment or update or this will be closed in 7 days.

@github-actions github-actions Bot added Stale and removed Stale labels Jul 22, 2026
@nharidas nharidas changed the title Added Cardinal class for Telugu TN Telugu TN: Cardinal Semiotic Class Aug 6, 2026

@folivoramanh folivoramanh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make sure both pytest and sparrowhawk test pass, and fix the problem of blank space

Comment thread nemo_text_processing/text_normalization/te/taggers/cardinal.py Outdated
Comment thread nemo_text_processing/text_normalization/te/taggers/cardinal.py Outdated
Comment thread nemo_text_processing/text_normalization/te/taggers/cardinal.py
Comment thread nemo_text_processing/text_normalization/te/taggers/word.py Outdated
Comment thread nemo_text_processing/text_normalization/te/graph_utils.py Outdated
Comment thread nemo_text_processing/text_normalization/normalize.py
Comment thread Jenkinsfile Outdated
Comment thread Jenkinsfile Outdated
Comment thread tests/nemo_text_processing/te/test_cardinal.py Outdated
@nharidas
nharidas force-pushed the feat-cardinal-te branch 2 times, most recently from 9d8b75f to b224888 Compare August 19, 2026 06:46
@nharidas

Copy link
Copy Markdown
Author

Thanks for the review. I’ve addressed the comments on this PR.

  • Removed Telugu post-processing (no PostProcessingFst for te; tests use post_process=False)
  • Dropped custom add_weight; competing *1 forms are split instead of using a -0.2 bias
  • Deleted teens_and_ties_thousand.tsv; combining ఒక is composed from ties
  • magnitudes.tsv is loaded with load_labels
  • Digit/zero classes come from digit.tsv / zero.tsv (no hardcoded NEMO_TE_DIGIT)
  • Word tagger simplified to NEMO_NOT_SPACE
  • Jenkinsfile: removed the commented TE ITN stage
  • Tests: assert pred == expected (no .strip())

pytest for Telugu cardinals is passing. Sparrowhawk tests are still pending; I’ll update this PR once they are run.

@nharidas
nharidas requested a review from folivoramanh August 19, 2026 07:51

@folivoramanh folivoramanh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • add init file in data/ (currently only in data/numbers/)

Comment thread Jenkinsfile Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check this example and see that if you can build a reusable component for thousand/million/billion/trillion
https://github.com/NVIDIA/NeMo-text-processing/blob/main/nemo_text_processing/text_normalization/es/taggers/cardinal.py

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pointer. I reviewed, es/taggers/cardinal.py.

Spanish works with one reusable block because it uses the Western scale: each step adds 6 zeros (thousand → million → billion → trillion) and reuses the same 3-digit pattern with a new magnitude word.

Telugu uses the Indian numbering system, which is structured differently:

  • 1,000 → thousand
  • 1,00,000 → lakh (not “hundred thousand”)
  • 1,00,00,000 → crore
  • higher tiers (arab, kharab, nil, padma, shankh) with different digit groupings

On top of that, the spoken form is not one fixed word per magnitude. It depends on position in the number and what follows, for example:

  • singular vs plural / “before” forms (e.g. వేల vs వేలు, కోటి vs కోట్లు)
  • special “one” form when combining (ఒక in 21, 31, … vs ఒకటి for standalone 1)
  • prefix variants (e.g. నూట vs వంద for hundreds)
  • special lakh form (లక్షా)

So we cannot drop in one ES-style thousand/million/billion component, both the scale and the word forms change at each tier.

This current code already reuses shared logic where the grammar allows it:

  • build_group, create_graph_suffix, create_larger_number_graph
  • magnitude ladders (thousand → lakh → crore → …)
  • shared strings in magnitudes.tsv

The longer code reflects real Telugu morphology, not unnecessary duplication. Collapsing it into one Spanish-style block would risk wrong readings (e.g. 21 → ఇరవై ఒక, not ఇరవై ఒకటి).

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it uses the Indian numbering system, are we building the graph in the same way that the Hindi graph does?

@nharidas nharidas Sep 2, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mgrafu Yes, same Indian numbering system and the same overall graph structure as Hindi.
Both use the tiered scale (hundred → thousand → lakh → crore → arab/kharab/nil/…) with the same create_graph_suffix / create_larger_number_graph pattern and the same 2+3 digit grouping.
Telugu extends that base for language-specific morphology: singular/plural/before magnitude forms, special “one” handling (ఒక vs ఒకటి), and hundred prefixes (నూట vs వంద), driven by TSVs and build_group ladders. Hindi uses a simpler one-suffix-per-tier model.

@nharidas

Copy link
Copy Markdown
Author

Thanks for the follow-up review.

Changes Made:

  • Added nemo_text_processing/text_normalization/te/data/init.py
  • Simplified Jenkinsfile to a single L0: TE TN grammars stage (removed the parallel Create TE TN/ITN Grammars wrapper)
  • Expanded cardinal tests to cover large magnitudes, including 999999999999999 (15 digits) and up to 19-digit shankh-scale numbers, plus Telugu digits, in-sentence, leading zeros, and negatives
  • Negative numbers: verbalizer now outputs “మైనస్” for the sign (Telugu localized form, same pattern as Spanish “menos”), instead of English “minus”. Test cases updated accordingly.

Testing:

  • Cardinal pytest test cases passed
  • Sparrowhawk “testTNCardinal” passed

Re: Spanish reusable component: replied in the thread with details.

@nharidas
nharidas requested a review from folivoramanh August 22, 2026 15:44
@folivoramanh
folivoramanh requested a review from mgrafu August 28, 2026 03:45
Comment thread tests/nemo_text_processing/te/test_sparrowhawk_normalization.sh Outdated
Comment thread nemo_text_processing/text_normalization/te/taggers/punctuation.py
Comment thread nemo_text_processing/text_normalization/te/taggers/cardinal.py Outdated
nharidas added a commit to nharidas/NeMo-text-processing that referenced this pull request Sep 2, 2026
Rebased onto staging/telugu_tn_v1 with TE-only changes for PR NVIDIA#447.

Signed-off-by: Haridas Nishitha <nharidas@nvidia.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@nharidas

nharidas commented Sep 2, 2026

Copy link
Copy Markdown
Author

Thanks for the review.

Changes Made:

  • Expanded test_cases_cardinal.txt with internal-zero and separator coverage (Indian and Western grouping)
  • Added separator handling in cardinal.py (indian_grouping / western_grouping) to strip commas before normalization
  • Moved hardcoded inputs to TSV files.
  • Removed unused code in test_sparrowhawk_normalization.sh

Testing:

  • Cardinal pytest test cases passed
  • Sparrowhawk testTNCardinal passed

@nharidas
nharidas requested a review from mgrafu September 2, 2026 09:35
"""
Finite state transducer for classifying cardinals, e.g.
-౨౩ -> cardinal { negative: "true" integer: "ఇరవై మూడు" }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's document what is the max magnitude for large numbers covered in the grammar

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mgrafu Done, documented in the CardinalFst class docstring.

@@ -0,0 +1,89 @@
4 నాలుగులు~నాలుగు నాలుగులు
౬ మంది ఆటగాళ్లు బయటకు~ఆరు మంది ఆటగాళ్లు బయటకు

@mgrafu mgrafu Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's review add these test cases:

50000000001~ఐదు వేల కోట్ల ఒకటి
2100000000000~రెండు లక్షల పది వేల కోట్లు
210000000000000~రెండు కోట్ల పది లక్షల కోట్లు
2200000000000000~ఇరవై రెండు కోట్ల కోట్లు
10000000000000000~వంద కోట్ల కోట్లు
10200000000000000~నూట రెండు కోట్ల కోట్లు
990000000000000000~తొమ్మిది వేల తొమ్మిది వందల కోట్ల కోట్లు
9999999990000000~తొంభై తొమ్మిది కోట్ల తొంభై తొమ్మిది లక్షల తొంభై తొమ్మిది వేల తొమ్మిది వందల తొంభై తొమ్మిది కోట్లు
99999999999999999~తొమ్మిది వందల తొంభై తొమ్మిది కోట్ల తొంభై తొమ్మిది లక్షల తొంభై తొమ్మిది వేల తొమ్మిది వందల తొంభై తొమ్మిది కోట్ల తొంభై తొమ్మిది లక్షల తొంభై తొమ్మిది వేల తొమ్మిది వందల తొంభై తొమ్మిది
12345678901234567~నూట ఇరవై మూడు కోట్ల నలభై ఐదు లక్షల అరవై ఏడు వేల ఎనిమిది వందల తొంభై కోట్ల పన్నెండు లక్షల ముప్పై నాలుగు వేల ఐదు వందల అరవై ఏడు
2100000000000000~ఇరవై ఒక కోటి కోట్లు
20100000000000000~రెండు వందల ఒక కోటి కోట్లు
99000000000000~తొంభై తొమ్మిది లక్షల కోట్లు
990000000000000~తొమ్మిది కోట్ల తొంభై లక్షల కోట్లు
1010000000~నూట ఒక కోటి
1012345567~నూట ఒక కోటి ఇరవై మూడు లక్షల నలభై ఐదు వేల ఐదు వందల అరవై ఏడు
1,01,00,00,000~నూట ఒక కోటి
2010000000~రెండు వందల ఒక కోటి
9010000000~తొమ్మిది వందల ఒక కోటి
1020000000~నూట రెండు కోట్లు
12౩45~12౩45
1౨345~1౨345
౧2345~౧2345
1234౫~1234౫
123౪56789~123౪56789
14~పద్నాలుగు
15~పదిహేను
16~పదహారు
19~పంతొమ్మిది
114~నూట పద్నాలుగు
1015~వెయ్యి పదిహేను
216~రెండు వందల పదహారు
౧౪~పద్నాలుగు
౧౯~పంతొమ్మిది
999999999~తొంభై తొమ్మిది కోట్ల తొంభై తొమ్మిది లక్షల తొంభై తొమ్మిది వేల తొమ్మిది వందల తొంభై తొమ్మిది
220000000~ఇరవై రెండు కోట్లు
210000000~ఇరవై ఒక కోటి
900000000~తొంభై కోట్లు
123456789~పన్నెండు కోట్ల ముప్పై నాలుగు లక్షల యాభై ఆరు వేల ఏడు వందల ఎనభై తొమ్మిది
1000~వెయ్యి
100000~లక్ష
10000000~కోటి
౧౦౦౦~వెయ్యి
౧౦౦౦౦౦~లక్ష
౧౦౦౦౦౦౦౦~కోటి
౧౦౧~నూట ఒకటి
౧౧౧~నూట పదకొండు
౧౩౦~నూట ముప్పై
21000~ఇరవై ఒక వెయ్యి
31000~ముప్పై ఒక వెయ్యి
91000~తొంభై ఒక వెయ్యి
21500~ఇరవై ఒక వేల ఐదు వందలు
౨౧౦౦౦~ఇరవై ఒక వెయ్యి
0~సున్నా
౦~సున్నా
-0~మైనస్ సున్నా
99999999999999999999~99999999999999999999
1,0000~ఒకటి , సున్నా సున్నా సున్నా సున్నా
12,34~పన్నెండు , ముప్పై నాలుగు
1,2,3~ఒకటి , రెండు , మూడు

should these also be changed?

Line 48 1000000000000000: పది కోటి కోట్లు → పది కోట్ల కోట్లు
Line 49 999999999999999999: change the first కోటి to కోట్ల
Line 50 9999999999999999999: change the first కోటి to కోట్ల
Line 53 (Telugu-digit form of line 50): same single-word change

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mgrafu Have added the test cases to unit tests. Fixed the singular and plural forms correction for large numbers.

Comment thread Jenkinsfile Outdated
}
}
}
stage('L0: TE TN grammars') {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TE stage is missing the branch guard and parallel wrapper every other language has

L0: TE TN grammars is declared at the top level of stages, between the HI block (ends line 115) and L0: Create DE/ES TN/ITN Grammars (line 122). Every other L0 grammar-creation stage in this file is a guarded wrapper stage containing a parallel block:

stage('L0: Create HI TN/ITN Grammars') {
  when {
    anyOf {
      branch 'main'
      branch 'staging/**'
      branch 'staging_*'
      changeRequest target: 'main'
    }
  }
  failFast true
  parallel { ... }
}

As written, the TE stage has no when guard and no failFast, so it builds the Telugu grammar unconditionally on every branch and every PR, serially, adding wall-clock time to builds that don't touch Telugu. Suggested replacement for lines 116–120:

stage('L0: Create TE TN Grammars') {
  when {
    anyOf {
      branch 'main'
      branch 'staging/**'
      branch 'staging_*'
      changeRequest target: 'main'
    }
  }
  failFast true
  parallel {
    stage('L0: TE TN grammars') {
      steps {
        sh 'CUDA_VISIBLE_DEVICES="" python nemo_text_processing/text_normalization/normalize.py --lang=te --text="౧" --cache_dir ${TE_TN_CACHE}'
      }
    }
  }
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mgrafu Done.

Comment thread Jenkinsfile Outdated
JA_TN_CACHE='/home/jenkins/TestData/text_norm/ci/grammars/10-17-24-1'
HI_TN_CACHE='/home/jenkins/TestData/text_norm/ci/grammars/06-04-26-5'
KO_TN_CACHE='/home/jenkins/TestData/text_norm/ci/grammars/06-04-25-6'
KO_TN_CACHE='/home/jenkins/TestData/text_norm/ci/grammars/07-29-26-1'

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please don't change cache date for other languages -- rebase to target if needed

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mgrafu Done.

@@ -0,0 +1,13 @@
# Copyright (c) 2026, NVIDIA CORPORATION. All rights reserved.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should also have tests for word and punctuation

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mgrafu Done, added dedicated word and punctuation TN tests

nharidas and others added 8 commits September 9, 2026 13:56
Rebased onto staging/telugu_tn_v1 with TE-only changes for PR NVIDIA#447.

Signed-off-by: Haridas Nishitha <nharidas@nvidia.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Haridas Nishitha <nharidas@nvidia.com>
Use TSV-based digit definitions in graph_utils, restore load_labels in
utils, and update Jenkinsfile with correct cache paths and TE CI stages.

Signed-off-by: Haridas Nishitha <nharidas@nvidia.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Haridas Nishitha <nharidas@nvidia.com>
for more information, see https://pre-commit.ci

Signed-off-by: Haridas Nishitha <nharidas@nvidia.com>
tokenize_and_classify passed a punctuation argument that WordFst no
longer accepts after the staging rebase.

Signed-off-by: Haridas Nishitha <nharidas@nvidia.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Haridas Nishitha <nharidas@nvidia.com>
Signed-off-by: Haridas Nishitha <nharidas@nvidia.com>
Signed-off-by: Haridas Nishitha <nharidas@nvidia.com>
for more information, see https://pre-commit.ci

Signed-off-by: Haridas Nishitha <nharidas@nvidia.com>
Signed-off-by: Haridas Nishitha <nharidas@nvidia.com>
Signed-off-by: Haridas Nishitha <nharidas@nvidia.com>
Signed-off-by: Haridas Nishitha <nharidas@nvidia.com>
@nharidas

nharidas commented Sep 9, 2026

Copy link
Copy Markdown
Author

@mgrafu Thanks for the review.

Changes Made:

  • Documented max magnitude coverage in the CardinalFst docstring (up to 19 digits / shankh / శంఖ, 10¹⁷; larger values via composed కోటి groups)
  • Fixed singular/plural కోటి vs కోట్ల for large numbers.
  • Expanded test_cases_cardinal.txt with the suggested large-number, mixed-script, separator, and edge cases
  • Updated Jenkins: guarded TE L0 grammar build (when + failFast + parallel) and L1 TE pytest stage with TE_TN_CACHE
  • Added word and punctuation unit tests (test_word.py, test_punctuation.py + corresponding test_cases_*.txt)
  • Updated test_sparrowhawk_normalization.sh to run word and punctuation Sparrowhawk suites

Testing:

  • Cardinal, word, and punctuation pytest cases passed
  • Sparrowhawk testTNCardinal, testTNWord, and testTNPunctuation passed

@nharidas
nharidas requested a review from mgrafu September 9, 2026 09:27
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.

3 participants