PyThaiNLP / PyThaiNLP/pythainlp
bug: Seq2Seq teacher forcing disabled by unconditional overwrite — intentional?
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 1.2k
- Forks
- 304
- Avg merge
- 2d 14h
- Merged PRs (30d)
- 14
Description
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:
- fd4b6676 (2019-08-02) — Original: teacher forcing with ternary expression. Correct.
- dd6410d4 (2022-10-24) — Removed teacher forcing, left
decoder_input = topi.detach(). - d48dc503 (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)
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in pythainlp/transliterate/thai2rom.py at Seq2Seq.forward() and inspect the teacher-forcing block around lines 421-426. Compare commits fd4b6676, dd6410d4, and d48dc503, then use the inspect-based reproduction to confirm the current behavior. Done means the intended training behavior is established and the unconditional overwrite or dead block is resolved accordingly.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- machine-learning
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 68/100