diff --git a/CHANGELOG.md b/CHANGELOG.md index 75b064033..ad18275d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,21 @@ and this project adheres to ## [Unreleased] +### Fixed + +- `pythainlp.transliterate.romanize` with the `thai2rom` and `thai2rom_onnx` + engines: greedy decoding could get stuck in a cycle and run to the hard + 100-character length cap, returning strings like + `krungtheppaaaa...aaaa` for inputs such as `กรุงเทพฯ`, `ฯลฯ`, or long + Pali/Sanskrit-derived compounds. Decoding now stops as soon as a short + output cycle repeats 3 times in a row. (issue #1403) +- `pythainlp.transliterate.thaig2p.transliterate`: found while fixing + issue #1403, the same greedy-decoding `Seq2Seq` loop as `thai2rom` + could get stuck in a repetition cycle and run to the hard 100-character + cap for the same class of inputs (e.g. `สตรีเศรษฐบุตรบำเพ็ญ`, and long + unsegmented multi-word phrases). Fixed with the same cycle-detection + guard used for `thai2rom`. + ## Changed - Improve guardrails in `check_sara()` and `nighit()` diff --git a/pythainlp/transliterate/_repetition.py b/pythainlp/transliterate/_repetition.py new file mode 100644 index 000000000..ac5d070f8 --- /dev/null +++ b/pythainlp/transliterate/_repetition.py @@ -0,0 +1,57 @@ +# SPDX-FileCopyrightText: 2016-2026 PyThaiNLP Project +# SPDX-FileType: SOURCE +# SPDX-License-Identifier: Apache-2.0 +"""Cycle detection for greedy seq2seq decoding. + +Greedy decoding (picking the highest-probability token at each step, +with no repetition penalty or n-gram blocking) can get the decoder +stuck in a loop: the attention mechanism repeatedly attends to the +same input position and keeps emitting the same short token sequence +until the hard length limit is hit. This module detects such a cycle +as it forms so a caller can stop decoding early instead of returning +an unbounded run of repeated characters. + +See: https://github.com/PyThaiNLP/pythainlp/issues/1403 +""" + +from __future__ import annotations + +from typing import List, Optional + +__all__: List[str] = ["find_trailing_repeat_period"] + + +def find_trailing_repeat_period( + tokens: List[int], + min_period: int = 1, + max_period: int = 12, + min_repeats: int = 3, +) -> Optional[int]: + """Detect a short cycle repeating at the end of a token sequence. + + Checks period lengths from ``min_period`` to ``max_period`` + (smallest first) and returns the first period whose last + ``min_repeats`` copies at the end of ``tokens`` are identical. + + :param tokens: sequence of decoded token ids, in generation order + :param min_period: shortest cycle length to check, in tokens + :param max_period: longest cycle length to check, in tokens + :param min_repeats: number of consecutive copies of the cycle + required at the end of ``tokens`` to count as a repetition loop + :return: the period of the detected cycle, or None if no trailing + cycle of at least ``min_repeats`` copies is found + :rtype: Optional[int] + """ + n = len(tokens) + for period in range(min_period, max_period + 1): + window = period * min_repeats + if n < window: + continue + segment = tokens[-window:] + first = segment[:period] + if all( + segment[i * period : (i + 1) * period] == first + for i in range(1, min_repeats) + ): + return period + return None diff --git a/pythainlp/transliterate/thai2rom.py b/pythainlp/transliterate/thai2rom.py index 194503416..39a3d3f66 100644 --- a/pythainlp/transliterate/thai2rom.py +++ b/pythainlp/transliterate/thai2rom.py @@ -13,6 +13,7 @@ from torch import nn from pythainlp.corpus import get_corpus_path +from pythainlp.transliterate._repetition import find_trailing_repeat_period if TYPE_CHECKING: from typing import Dict @@ -23,6 +24,11 @@ _MODEL_NAME: str = "thai2rom-pytorch-attn" +# Minimum consecutive copies of a repeating cycle that marks the greedy +# decoder as stuck in a loop. See: find_trailing_repeat_period() and +# https://github.com/PyThaiNLP/pythainlp/issues/1403 +_REPEAT_MIN_CYCLES: int = 3 + class ThaiTransliterator: __model_filename: str @@ -410,6 +416,7 @@ def forward( max_source_len = encoder_outputs.size(1) mask = self.create_mask(source_seq[:, 0:max_source_len]) + generated_tokens: list[int] = [] for di in range(max_len): decoder_output, decoder_hidden, _ = self.decoder( decoder_input, decoder_hidden, encoder_outputs, mask @@ -426,8 +433,23 @@ def forward( else: decoder_input = topi.detach() - if inference and decoder_input == end_token: - return outputs[:di] + if inference: + if decoder_input == end_token: + return outputs[:di] + + # Greedy decoding has no repetition penalty, so a trapped + # attention pattern can loop forever instead of emitting + # . Stop as soon as a short cycle repeats, keeping + # one copy of it, rather than running to max_len. + generated_tokens.append(int(decoder_input.item())) + period = find_trailing_repeat_period( + generated_tokens, min_repeats=_REPEAT_MIN_CYCLES + ) + if period is not None: + cutoff = len(generated_tokens) - period * ( + _REPEAT_MIN_CYCLES - 1 + ) + return outputs[:cutoff] return outputs diff --git a/pythainlp/transliterate/thai2rom_onnx.py b/pythainlp/transliterate/thai2rom_onnx.py index e11e614ce..9b2471137 100644 --- a/pythainlp/transliterate/thai2rom_onnx.py +++ b/pythainlp/transliterate/thai2rom_onnx.py @@ -11,6 +11,7 @@ from onnxruntime import InferenceSession from pythainlp.corpus import get_corpus_path +from pythainlp.transliterate._repetition import find_trailing_repeat_period if TYPE_CHECKING: from typing import Dict, List @@ -22,6 +23,11 @@ _MODEL_DECODER_NAME: str = "thai2rom_decoder_onnx" _MODEL_CONFIG_NAME: str = "thai2rom_config_onnx" +# Minimum consecutive copies of a repeating cycle that marks the greedy +# decoder as stuck in a loop. See: find_trailing_repeat_period() and +# https://github.com/PyThaiNLP/pythainlp/issues/1403 +_REPEAT_MIN_CYCLES: int = 3 + class ThaiTransliterator_ONNX: def __init__(self) -> None: @@ -231,6 +237,7 @@ def run( max_source_len = encoder_outputs.shape[1] mask = self.create_mask(source_seq[:, 0:max_source_len]) + generated_tokens: List[int] = [] for di in range(max_len): decoder_output_raw, decoder_hidden = self.decoder.run( input_feed={ @@ -254,6 +261,20 @@ def run( if decoder_input.item() == end_token: return outputs[:di] + # Greedy decoding has no repetition penalty, so a trapped + # attention pattern can loop forever instead of emitting + # . Stop as soon as a short cycle repeats, keeping one + # copy of it, rather than running to max_len. + generated_tokens.append(int(decoder_input.item())) + period = find_trailing_repeat_period( + generated_tokens, min_repeats=_REPEAT_MIN_CYCLES + ) + if period is not None: + cutoff = len(generated_tokens) - period * ( + _REPEAT_MIN_CYCLES - 1 + ) + return outputs[:cutoff] + return outputs diff --git a/pythainlp/transliterate/thaig2p.py b/pythainlp/transliterate/thaig2p.py index a2df19022..b9f72afad 100644 --- a/pythainlp/transliterate/thaig2p.py +++ b/pythainlp/transliterate/thaig2p.py @@ -15,6 +15,7 @@ from torch import nn from pythainlp.corpus import get_corpus_path +from pythainlp.transliterate._repetition import find_trailing_repeat_period if TYPE_CHECKING: from numpy.typing import NDArray @@ -25,6 +26,11 @@ _MODEL_NAME: str = "thai-g2p" +# Minimum consecutive copies of a repeating cycle that marks the greedy +# decoder as stuck in a loop. See: find_trailing_repeat_period() and +# https://github.com/PyThaiNLP/pythainlp/issues/1403 +_REPEAT_MIN_CYCLES: int = 3 + class ThaiG2P: """ @@ -429,6 +435,7 @@ def forward( max_source_len = encoder_outputs.size(1) mask = self.create_mask(source_seq[:, 0:max_source_len]) + generated_tokens: list[int] = [] for di in range(max_len): decoder_output, decoder_hidden, _ = self.decoder( decoder_input, decoder_hidden, encoder_outputs, mask @@ -446,8 +453,23 @@ def forward( else topi.detach() ) - if inference and decoder_input == end_token: - return outputs[:di] + if inference: + if decoder_input == end_token: + return outputs[:di] + + # Greedy decoding has no repetition penalty, so a trapped + # attention pattern can loop forever instead of emitting + # . Stop as soon as a short cycle repeats, keeping + # one copy of it, rather than running to max_len. + generated_tokens.append(int(decoder_input.item())) + period = find_trailing_repeat_period( + generated_tokens, min_repeats=_REPEAT_MIN_CYCLES + ) + if period is not None: + cutoff = len(generated_tokens) - period * ( + _REPEAT_MIN_CYCLES - 1 + ) + return outputs[:cutoff] return outputs diff --git a/tests/core/test_transliterate.py b/tests/core/test_transliterate.py index 757d87a4d..dcee3f9d7 100644 --- a/tests/core/test_transliterate.py +++ b/tests/core/test_transliterate.py @@ -5,6 +5,7 @@ import unittest from pythainlp.transliterate import pronunciate_pali, romanize, transliterate +from pythainlp.transliterate._repetition import find_trailing_repeat_period BASIC_TESTS = { None: "", @@ -190,3 +191,36 @@ def test_pronunciate_pali(self): self.assertEqual( pronunciate_pali("พฺราหฺมณ"), "พราหมะณะ" ) + + +class RepetitionCycleTestCase(unittest.TestCase): + """Tests for the greedy-decoding cycle detector. + + See: https://github.com/PyThaiNLP/pythainlp/issues/1403 + """ + + def test_no_cycle(self): + self.assertIsNone(find_trailing_repeat_period([])) + self.assertIsNone(find_trailing_repeat_period([1, 2, 3, 4, 5])) + self.assertIsNone(find_trailing_repeat_period([1, 1, 2, 2])) + + def test_single_char_cycle(self): + # e.g. the "aaaa..." tail seen for "กรุงเทพฯ" + self.assertEqual(find_trailing_repeat_period([9, 1, 1, 1]), 1) + self.assertIsNone(find_trailing_repeat_period([1, 1])) + + def test_multi_char_cycle(self): + # e.g. the "botbotbot..." tail seen for "ราษฎรบำรุง" + tokens = [9, 8, 7, 1, 2, 3, 1, 2, 3, 1, 2, 3] + self.assertEqual(find_trailing_repeat_period(tokens), 3) + + def test_longer_period_requires_larger_max_period(self): + period = list(range(8)) + tokens = period * 3 + self.assertIsNone(find_trailing_repeat_period(tokens, max_period=5)) + self.assertEqual(find_trailing_repeat_period(tokens, max_period=8), 8) + + def test_min_repeats_threshold(self): + tokens = [1, 2, 1, 2] # only repeats twice + self.assertIsNone(find_trailing_repeat_period(tokens, min_repeats=3)) + self.assertEqual(find_trailing_repeat_period(tokens, min_repeats=2), 2) diff --git a/tests/noauto_onnx/testn_transliterate_onnx.py b/tests/noauto_onnx/testn_transliterate_onnx.py index 0576e3f16..10e9cdcab 100644 --- a/tests/noauto_onnx/testn_transliterate_onnx.py +++ b/tests/noauto_onnx/testn_transliterate_onnx.py @@ -39,3 +39,18 @@ def test_thai2rom_onnx_mixed_text(self): result = romanize("ภาษาไทย") self.assertIsInstance(result, str) self.assertGreater(len(result), 0) + + def test_thai2rom_onnx_no_repetition_runaway(self): + # Regression test for https://github.com/PyThaiNLP/pythainlp/issues/1403 + # Greedy decoding used to get stuck in a repetition loop and run + # to the hard _maxlength=100 cap for these inputs. + from pythainlp.transliterate.thai2rom_onnx import romanize + + for word in ( + "กรุงเทพฯ", + "ฯลฯ", + "ราษฎรบำรุง", + "สตรีเศรษฐบุตรบำเพ็ญ", + ): + result = romanize(word) + self.assertLess(len(result), 100) diff --git a/tests/noauto_torch/testn_transliterate_torch.py b/tests/noauto_torch/testn_transliterate_torch.py index 5d7268e35..e49197e0a 100644 --- a/tests/noauto_torch/testn_transliterate_torch.py +++ b/tests/noauto_torch/testn_transliterate_torch.py @@ -36,6 +36,21 @@ def test_thai2rom_empty_string(self): result = romanize("") self.assertIsInstance(result, str) + def test_thai2rom_no_repetition_runaway(self): + # Regression test for https://github.com/PyThaiNLP/pythainlp/issues/1403 + # Greedy decoding used to get stuck in a repetition loop and run + # to the hard _maxlength=100 cap for these inputs. + from pythainlp.transliterate.thai2rom import romanize + + for word in ( + "กรุงเทพฯ", + "ฯลฯ", + "ราษฎรบำรุง", + "สตรีเศรษฐบุตรบำเพ็ญ", + ): + result = romanize(word) + self.assertLess(len(result), 100) + def test_thaig2p_returns_string(self): from pythainlp.transliterate.thaig2p import transliterate @@ -53,6 +68,24 @@ def test_thaig2p_model_loaded(self): self.assertIn("", g2p._target_char_to_ix) self.assertIn("", g2p._target_char_to_ix) + def test_thaig2p_no_repetition_runaway(self): + # Regression test for https://github.com/PyThaiNLP/pythainlp/issues/1403 + # thaig2p.py shares the same greedy-decoding Seq2Seq loop as + # thai2rom.py and could get stuck in a repetition loop, running + # to the hard _maxlength=100 cap for these inputs. + from pythainlp.transliterate.thaig2p import transliterate + + for word in ( + "กรุงเทพฯ", + "ฯลฯ", + "ราษฎรบำรุง", + "สตรีเศรษฐบุตรบำเพ็ญ", + "เอ็มเอฟซีบัญชีเพื่อการชำระค่ารับซื้อคืน", + "บัญชีเพื่อการชำระค่าขายคืนหน่วยลงทุน", + ): + result = transliterate(word) + self.assertLess(len(result), 100) + def test_thaig2p_v2_returns_string(self): from pythainlp.transliterate.thaig2p_v2 import transliterate