Skip to content

Fix romanize() returning empty string / literal <PAD> for thai2rom engines - #1351

Closed
wannaphong with Copilot wants to merge 3 commits into
devfrom
copilot/fix-romanize-empty-string
Closed

Fix romanize() returning empty string / literal <PAD> for thai2rom engines#1351
wannaphong with Copilot wants to merge 3 commits into
devfrom
copilot/fix-romanize-empty-string

Conversation

Copilot AI commented Mar 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

romanize() with engine="thai2rom" or engine="thai2rom_onnx" returned either an empty string or the literal token string "<PAD>" instead of the correct romanization.

What was wrong

Two bugs, both engines:

  1. Wrong fallback on empty model output — when the seq2seq model immediately predicts <end> (no characters generated), the code set target = ["<PAD>"] and returned the 5-character string "<PAD>" instead of "".
  2. Special tokens leaked into output — the raw model output was joined without filtering; any <PAD>, <start>, or <end> token that appeared would be included literally in the result.

Additionally, thai2rom_onnx had three pre-existing bugs now fixed (via merged PR #1349 already in the base branch): ONNX encoder TopK K-input was a rank-0 scalar rejected by newer ONNX Runtime; ix_to_char/ix_to_target_char JSON keys are strings but were looked up as int, causing KeyError on every decode step; loop-termination decoder_input == end_token compared a NumPy array to a scalar.

How this fixes it

  • thai2rom.py / thai2rom_onnx.py: Return "" immediately when target_tensor_logits is empty. For non-empty output, filter out <PAD>, <start>, <end> before joining.
  • _SPECIAL_TARGET_TOKENS module-level constant (shared pattern in both modules) drives the filter.
  • Tests (tests/extra/testx_transliterate.py): Added romanize("สมชาย") == "somchai" for both engines — the exact case from the bug report.
  • Tests (tests/noauto_onnx/testn_tokenize_onnx.py): Extended TransliterateONNXTestCaseN with comprehensive ONNX-specific test cases:
    • test_thai2rom_onnx_romanize: covers the bug-report case ("สมชาย" → "somchai"), common Thai words, and multi-word space-separated input.
    • test_thai2rom_onnx_edge_cases: verifies that empty string input returns "" and that no raw special tokens (<PAD>, <start>, <end>) appear in output. Run locally with python -m unittest tests.noauto_onnx.
from pythainlp.transliterate import romanize

romanize("สมชาย", engine="thai2rom")       # "somchai"  (was "" or "<PAD>")
romanize("สมชาย", engine="thai2rom_onnx")  # "somchai"  (was "" or error)

Your checklist for this pull request

  • Passed code styles and structures
  • Passed code linting checks and unit test
Original prompt

This section details on the original issue you should resolve

<issue_title>bug: romanize() returns empty string for both "thai2rom" and "thai2rom_onnx" engines</issue_title>
<issue_description>### Description

Calling romanize() with either engine="thai2rom" or engine="thai2rom_onnx"
returns an empty string

Expected results

"somchai"

Current results

"" (empty string)

Steps to reproduce

from pythainlp.transliterate import romanize

print(repr(romanize("สมชาย", engine="thai2rom")))       # Output: ''
print(repr(romanize("สมชาย", engine="thai2rom_onnx")))  # Output: ''

### PyThaiNLP version

 5.3.2

### Python version

3.12

### Operating system and version

Windows 11

### More info

_No response_

### Possible solution

_No response_

### Files

_No response_</issue_description>

<agent_instructions>Check thai2rom in windows</agent_instructions>

## Comments on the Issue (you are @copilot in this section)

<comments>
<comment_new><author>@wannaphong</author><body>
@gripfy Thank you for your report. I try to reproduce the bug and it look thai2rom is fine but thai2rom_onnx has a bug ##1348. Can you tell me more details?</body></comment_new>
</comments>

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

@wannaphong
Fix romanize() returning empty/wrong string for thai2rom and thai2rom…
ef5d958 
…_onnx

- Return "" instead of literal "<PAD>" when model produces no output
- Filter special tokens (<PAD>, <start>, <end>) from romanized output
- Add romanize("สมชาย") test case for both engines (issue #1346)
- Document fixes in CHANGELOG

Co-authored-by: wannaphong <8536487+wannaphong@users.noreply.github.com>
Agent-Logs-Url: https://github.com/PyThaiNLP/pythainlp/sessions/4a12e2fc-ac10-4186-9a10-f71db25d87bf
Copilot AI changed the title [WIP] Fix romanize() returning empty string for thai2rom engines Fix romanize() returning empty string / literal <PAD> for thai2rom engines Mar 21, 2026
Copilot AI requested a review from wannaphong March 21, 2026 06:02
@bact

bact commented Mar 21, 2026

Copy link
Copy Markdown
Member

@copilot @wannaphong note that ONNX-dependent functions are in "noauto" test suite and will not be run in the test CI (due to its large dependency size and long testing time). It has to be tested locally (python -m unittest tests.noauto_onnx).

The test for thai2rom_onnx is specifically at:

pythainlp/tests/noauto_onnx/testn_tokenize_onnx.py

Lines 83 to 91 in 715cc06

class TransliterateONNXTestCaseN(unittest.TestCase):
"""Tests for ONNX-based transliteration (requires onnxruntime)"""
def test_thai2rom_onnx(self):
from pythainlp.transliterate.thai2rom_onnx import romanize
result = romanize("สวัสดี")
self.assertIsInstance(result, str)
self.assertGreater(len(result), 0)

We can extend this test case to cover more cases, including edge cases.

(We should also later move it to testn_transliterate_onnx.py, to put the transliterate test cases in transliterate test file)

@bact
Extend TransliterateONNXTestCaseN in testn_tokenize_onnx.py with comp…
79357d6 
…rehensive test cases

- Add test_thai2rom_onnx_romanize: covers somchai (bug-report case), common
  Thai words, and multi-word input
- Add test_thai2rom_onnx_edge_cases: verifies empty string returns "" and that
  no raw special tokens (<PAD>, <start>, <end>) appear in output

Co-authored-by: bact <128572+bact@users.noreply.github.com>
Agent-Logs-Url: https://github.com/PyThaiNLP/pythainlp/sessions/a5818628-bfa1-4601-a6ed-b076cab9b8f9

Copilot AI commented Mar 21, 2026

Copy link
Copy Markdown
Contributor Author

@copilot @wannaphong note that ONNX-dependent functions are in "noauto" test suite and will not be run in the test CI (due to its large dependency size and long testing time). It has to be tested loca...

Extended TransliterateONNXTestCaseN in tests/noauto_onnx/testn_tokenize_onnx.py (commit 79357d6):

  • test_thai2rom_onnx_romanize: covers romanize("สมชาย") == "somchai" (the bug-report case), common words (แมว, บ้านไร่, สุนัข, นก, ความอิ่ม, สกุนต์, ชารินทร์), and multi-word space-separated input.
  • test_thai2rom_onnx_edge_cases: verifies empty string returns "" and that no raw special tokens (<PAD>, <start>, <end>) appear in the output.

The module-level import was changed from pythainlp.transliterate.thai2rom_onnx to pythainlp.transliterate.romanize(..., engine="thai2rom_onnx") to test the full public API. Moving to testn_transliterate_onnx.py can be done in a follow-up.

Copilot AI requested a review from bact March 21, 2026 08:05
Copilot finished work on behalf of bact March 21, 2026 08:05
@sonarqubecloud

sonarqubecloud Bot commented Mar 21, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@wannaphong

wannaphong commented Mar 21, 2026

Copy link
Copy Markdown
Member

@copilot @wannaphong note that ONNX-dependent functions are in "noauto" test suite and will not be run in the test CI (due to its large dependency size and long testing time). It has to be tested locally (python -m unittest tests.noauto_onnx).

The test for thai2rom_onnx is specifically at:

pythainlp/tests/noauto_onnx/testn_tokenize_onnx.py

Lines 83 to 91 in 715cc06

class TransliterateONNXTestCaseN(unittest.TestCase):
"""Tests for ONNX-based transliteration (requires onnxruntime)"""
def test_thai2rom_onnx(self):
from pythainlp.transliterate.thai2rom_onnx import romanize
result = romanize("สวัสดี")
self.assertIsInstance(result, str)
self.assertGreater(len(result), 0)

We can extend this test case to cover more cases, including edge cases.

(We should also later move it to testn_transliterate_onnx.py, to put the transliterate test cases in transliterate test file)

Yes but thai2rom (pytorch) should working without has output bug. It's weird.

Copilot AI commented Mar 21, 2026

Copy link
Copy Markdown
Contributor Author

@wannaphong The model claude-sonnet-4.6 is not available for your account. This can happen if the model was disabled by your organization's policy or if your Copilot plan doesn't include access to it.

You can try again without specifying a model (just @copilot) to use the default, or choose a different model from the model picker.

If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: 52de4aa0-f484-4e75-a52b-b991aafed628

5 similar comments

Copilot AI commented Mar 21, 2026

Copy link
Copy Markdown
Contributor Author

@wannaphong The model claude-sonnet-4.6 is not available for your account. This can happen if the model was disabled by your organization's policy or if your Copilot plan doesn't include access to it.

You can try again without specifying a model (just @copilot) to use the default, or choose a different model from the model picker.

If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: 52de4aa0-f484-4e75-a52b-b991aafed628

Copilot AI commented Mar 21, 2026

Copy link
Copy Markdown
Contributor Author

@wannaphong The model claude-sonnet-4.6 is not available for your account. This can happen if the model was disabled by your organization's policy or if your Copilot plan doesn't include access to it.

You can try again without specifying a model (just @copilot) to use the default, or choose a different model from the model picker.

If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: 52de4aa0-f484-4e75-a52b-b991aafed628

Copilot AI commented Mar 21, 2026

Copy link
Copy Markdown
Contributor Author

@wannaphong The model claude-sonnet-4.6 is not available for your account. This can happen if the model was disabled by your organization's policy or if your Copilot plan doesn't include access to it.

You can try again without specifying a model (just @copilot) to use the default, or choose a different model from the model picker.

If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: 52de4aa0-f484-4e75-a52b-b991aafed628

Copilot AI commented Mar 21, 2026

Copy link
Copy Markdown
Contributor Author

@wannaphong The model claude-sonnet-4.6 is not available for your account. This can happen if the model was disabled by your organization's policy or if your Copilot plan doesn't include access to it.

You can try again without specifying a model (just @copilot) to use the default, or choose a different model from the model picker.

If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: 52de4aa0-f484-4e75-a52b-b991aafed628

Copilot AI commented Mar 21, 2026

Copy link
Copy Markdown
Contributor Author

@wannaphong The model claude-sonnet-4.6 is not available for your account. This can happen if the model was disabled by your organization's policy or if your Copilot plan doesn't include access to it.

You can try again without specifying a model (just @copilot) to use the default, or choose a different model from the model picker.

If you want to contact GitHub about this error, please mention the following identifier so they can better serve you: 52de4aa0-f484-4e75-a52b-b991aafed628

@wannaphong

wannaphong commented Mar 21, 2026

Copy link
Copy Markdown
Member

Update #1346, I think it has fixed in #1349 follow #1346 (comment)

@wannaphong wannaphong closed this Mar 21, 2026
@bact
bact deleted the copilot/fix-romanize-empty-string branch March 21, 2026 12:06
@bact bact added this to the 5.3.3 milestone Mar 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wannaphong wannaphong Awaiting requested review from wannaphong
@bact bact Awaiting requested review from bact

Assignees

@wannaphong wannaphong

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

5.3.3

Development

Successfully merging this pull request may close these issues.

bug: romanize() returns empty string for both "thai2rom" and "thai2rom_onnx" engines

3 participants

@bact @wannaphong