Skip to content
This repository has been archived by the owner on Nov 22, 2022. It is now read-only.

Fix bug in copy_unk #964

Closed
wants to merge 1 commit into from

Conversation

ArmenAg
Copy link
Contributor

@ArmenAg ArmenAg commented Sep 9, 2019

Summary:
When the copy_unk flag is set to true. Any unk that is produced in the output of the Seq2Seq model is replaced by the token that was mapped to unk from the utterance. This is a easy way to get gains since outputs with unk are always wrong.

Looking at the old code for copying the unk token we see that TorchScript optimizes out the actual search of the unk token in utterance:

{F207887831}

This diff updates the code to produce the correct TorchScript Graph

{F207888470}

Differential Revision: D17213086

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Sep 9, 2019
Summary:
Pull Request resolved: facebookresearch#964

When the copy_unk flag is set to true. Any unk that is produced in the output of the Seq2Seq model is replaced by the token that was mapped to unk from the utterance. This is a easy way to get gains since outputs with unk are always wrong.

Looking at the old code for copying the unk token we see that TorchScript optimizes out the actual search of the unk token in utterance:

{F207887831}

This diff updates the code to produce the correct TorchScript Graph

{F207888470}

Reviewed By: arbabu123

Differential Revision: D17213086

fbshipit-source-id: 9f02f7dddf79671e96feb28b0581ded504cb69fc
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in b362c31.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed Do not delete this pull request or issue due to inactivity. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants