Ko TN: Add serial grammar and review fixes#457
Conversation
…handling Signed-off-by: Jinwoo Bae <bbae7050@gmail.com>
for more information, see https://pre-commit.ci
Signed-off-by: Jinwoo Bae <bbae7050@gmail.com>
…move post_processing dependency Signed-off-by: Jinwoo Bae <bbae7050@gmail.com>
for more information, see https://pre-commit.ci
Signed-off-by: Jinwoo Bae <bbae7050@gmail.com>
for more information, see https://pre-commit.ci
Signed-off-by: Jinwoo Bae <bbae7050@gmail.com>
for more information, see https://pre-commit.ci
tbartley94
left a comment
There was a problem hiding this comment.
Serial appears more appropriate as part of the cardinal class no? What's the rationale for a standalone class?
| @@ -1,4 +1,4 @@ | |||
| # Copyright (c) 2026, NVIDIA CORPORATION. All rights reserved. | |||
| # Copyright (c) 2025, NVIDIA CORPORATION. All rights reserved. | |||
| """ | ||
|
|
||
| def __init__(self, deterministic: bool = True): | ||
| super().__init__(name="serial", kind="classify", deterministic=deterministic) |
There was a problem hiding this comment.
don't believe serial is sparrowhawk compliant no?
| zero_map = pynini.cross("0", "영") | ||
| digit_ko = (digit | zero_map).optimize() | ||
|
|
||
| two_digits = digit_ko**2 |
There was a problem hiding this comment.
is there no issue with potential 0 leading strings?
There was a problem hiding this comment.
from what i'm seeing is that this is just replacing a single korean character. couldn't you compress the three together for simplicity?
I made it standalone because the digit-by-digit reading is triggered by context such as 번호 or 연락처, while regular numbers should remain standard cardinal readings. I agree the numeric logic is closer to cardinal, though. Would you prefer that I move it into CardinalFst and remove the standalone class? |
Take a gander if it's feasible to just make it in Cardinals and work from there. If it's too messy then we can keep it. I'm just trying to limit semiotic spread |
Sounds good. I’ll see if I can integrate it into the cardinal class cleanly and push the update. If it gets too messy, I’ll let you know. |
What does this PR do ?
Adds Korean TN serial grammar support for number-like strings with clear context signals such as 번호 and 연락처.
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.