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

Implementing abstraction to score final sequences in BeamSearch #5208

Merged
merged 7 commits into from
May 18, 2021

Conversation

danieldeutsch
Copy link
Contributor

Implements feature requests 2 and 3 from #5205 and closes #5113.

Changes proposed in this pull request:

  • Other libraries, such as transformers or fairseq pick the top outputs from beam search by taking the sequence with the highest average log probability per token. See here for the transformers implementation. The AllenNLP implementation takes the sequence with the best total log probability of the sequence. This change adds an abstraction called FinalSequenceScorer to decide how the final sequences will be scored. The default is SequenceLogProbabilityScorer, which assigns a score equal to sum of the log probabilities per token (i.e., the current implementation). This PR also includes LengthNormalizedSequenceLogProbabilityScorer which assigns a score equal to the average log probability per token.
  • The LengthNormalizedSequenceLogProbabilityScorer also has a length_penalty parameter, which will increase or decrease sequence scores based on their length. This is also included in the transformers beam search.

Notes

  • This change does not break the code, but it does change the semantics of the second tensor returned by the search method in BeamSearch. Previously it was the log probabilities of the sequences, but now it is the score that is returned by the FinalSequenceScorer class. The new default is to return what is currently returned, so this should not break any models.

Before submitting

  • I've read and followed all steps in the Making a pull request
    section of the CONTRIBUTING docs.
  • I've updated or added any relevant docstrings following the syntax described in the
    Writing docstrings section of the CONTRIBUTING docs.
  • If this PR fixes a bug, I've added a test that will fail without my fix.
  • If this PR adds a new feature, I've added tests that sufficiently cover my new functionality.

After submitting

  • All GitHub Actions jobs for my pull request have passed.
  • codecov/patch reports high test coverage (at least 90%).
    You can find this under the "Actions" tab of the pull request once the other checks have finished.

Copy link
Member

@epwalsh epwalsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also looks great! I appreciate the thorough documentation 🙂 Just a few minor comments

allennlp/nn/beam_search.py Show resolved Hide resolved
allennlp/nn/beam_search.py Outdated Show resolved Hide resolved
allennlp/nn/beam_search.py Outdated Show resolved Hide resolved
allennlp/nn/beam_search.py Outdated Show resolved Hide resolved
@epwalsh epwalsh enabled auto-merge (squash) May 17, 2021 21:30
auto-merge was automatically disabled May 18, 2021 01:21

Head branch was pushed to by a user without write access

Copy link
Member

@epwalsh epwalsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thank you @danieldeutsch!

@epwalsh epwalsh merged commit 3585c9f into allenai:main May 18, 2021
@danieldeutsch danieldeutsch deleted the sequence-scoring branch May 18, 2021 17:00
Abhishek-P pushed a commit to Abhishek-P/allennlp that referenced this pull request Aug 11, 2021
…lenai#5208)

* Implementing FinalSequenceScorer in BeamSearch

* Including the end token in the normalization

* Reformating

* Apply suggestions from code review

Co-authored-by: Pete <[email protected]>

* Sorting the sequences by the final scores

Co-authored-by: Pete <[email protected]>
Co-authored-by: Pete <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Length penalty for nn.beam_search.BeamSearch
2 participants