Skip to content

Fix decoder prefix preprocessing in alternatives mode - #2087

Draft
akira-in-tech wants to merge 1 commit into
OpenNMT:masterfrom
akira-in-tech:fix/alternative-prefix-embeddings
Draft

Fix decoder prefix preprocessing in alternatives mode#2087
akira-in-tech wants to merge 1 commit into
OpenNMT:masterfrom
akira-in-tech:fix/alternative-prefix-embeddings

Conversation

@akira-in-tech

Copy link
Copy Markdown

Summary

Fix decoder prefix prefill when return_alternatives forwards multiple tokens at decoding step 0.

For models that both start from a zero decoder embedding and scale embeddings (including Marian conversions), the previous sequence path zeroed the entire prefetched prefix and skipped scaling for every token. This made the alternatives distribution differ from normal step-by-step decoding.

The decoder now uses the existing sequence preprocessing behavior for multi-token input at step 0: only the first embedding is zeroed, while later prefix embeddings are scaled normally. Position encoding and decoder state still use the original decoding step.

Fixes #2014.

Tests

  • Added a deterministic synthetic Transformer regression test with start_from_zero_embedding=True and embedding scaling enabled.
  • Verified the new test fails before the fix (-0.6844518 vs expected -0.6613321) and passes after the fix.
  • python -m pytest python/tests/test_translator.py::test_return_alternatives_with_scaled_prefix -q: 1 passed.
  • Related C++ decoder and alternatives tests: 8 passed.
  • Full local C++ suite: 190 passed, 1 skipped, and 1 failure in Conv1DGroupNoBiasQuantized on the macOS Accelerate backend, outside the decoder code path changed here.
  • black --check, isort --check-only, flake8, and git diff --check pass for the changed files (using the repository's existing flake8 ignores).

AI disclosure

AI assistance was used to search for duplicate work, reason about the reported root cause, and help draft the patch and regression test. I reviewed the decoder flow, built and ran the before/after reproducer, checked the final diff, and remain responsible for the correctness and design of this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

return_alternatives next-pos tokens don't match top-probability tokens of sampling mode

1 participant