Skip to content

bug: Seq2Seq teacher forcing disabled by unconditional overwrite — intentional? #1390

@phoneee

Description

@phoneee

Description

In Seq2Seq.forward() (transliterate/thai2rom.py), the teacher forcing if-else block is immediately overwritten by an unconditional assignment:

# line 421-426
if teacher_force and target_seq is not None:
    decoder_input = target_seq[:, di].reshape(batch_size, 1)
else:
    decoder_input = topi.detach()

decoder_input = topi.detach()  # ← this always runs, overwriting the above

Line 426 makes the if-else block dead code — teacher_forcing_ratio has no effect.

History

Looking at git history, this appears to be an incomplete revert:

  1. fd4b667 (2019-08-02) — Original: teacher forcing with ternary expression. Correct.
  2. dd6410d (2022-10-24) — Removed teacher forcing, left decoder_input = topi.detach().
  3. d48dc50 (2022-10-25) — "fix: bringback original thai2rom code" — added back the if-else but did not remove the old line from step 2.

Question

Was removing teacher forcing in step 2 intentional? If the intent of step 3 was to restore it, then line 426 should be removed. The pre-trained model shipped with PyThaiNLP was trained before this change, so romanize(engine="thai2rom") still works — only retraining/fine-tuning is affected.

If teacher forcing is no longer needed (e.g. the model is only used for inference), the if-else block at lines 421-424 could be removed instead.

Steps to reproduce

import inspect
from pythainlp.transliterate.thai2rom import Seq2Seq

source = inspect.getsource(Seq2Seq.forward)
# Lines 421-426 show the if-else followed by unconditional overwrite

PyThaiNLP version

5.3.3

Files

  • pythainlp/transliterate/thai2rom.py (lines 421-426)

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugbugs in the library

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions