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

Fix "calling median on empty tensor" issue in MR #1322

Closed

Conversation

psuzhanhy
Copy link
Contributor

Summary:
Language model metrics reporter occassionally throws "median on empty tensor", e.g: f182865817

why?
We used to check the cross entropy tensor != 0 before calling mean/median on it, to exclude the calculation of the padding positions which is just 0, which possibly results in an empty tensor.

This diff checks the tensor posisions against pad_index instead of using 0.0:a batch can have close to 0 CE, or the predict word index happened to be pad_index.

Also a failed safe is added if the tensor[target != self.pad_index] is indeed empty.

Differential Revision: D21081502

Summary:
Language model metrics reporter occassionally throws "median on empty tensor", e.g: f182865817

why?
We used to check the cross entropy tensor != 0 before calling mean/median on it, to exclude the calculation of the padding positions which is just 0, which possibly results in an empty tensor.

This diff checks the tensor posisions against pad_index instead of using 0.0:a batch can have close to 0 CE, or the predict word index happened to be pad_index.

Also a failed safe is added if the tensor[target != self.pad_index] is indeed empty.

Differential Revision: D21081502

fbshipit-source-id: 685705897bd350b1ae3af6fae1e84725e045bb70
@facebook-github-bot facebook-github-bot added CLA Signed Do not delete this pull request or issue due to inactivity. fb-exported labels Apr 17, 2020
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D21081502

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 7db6094.

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. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants