Skip to content

Conversation

@secrettoad
Copy link
Contributor

This PR addresses #34843, adjusting the requirement to early stop during generation via beam search to ANY of the supplied criteria as opposed to ALL

Fixes #34843

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline,
    Pull Request section?
  • Was this discussed/approved via a Github issue or the forum? Please add a link
    to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?

Who can review?

@Rocketknight1 @gante

@secrettoad
Copy link
Contributor Author

I'll take a look at the failing tests

@secrettoad secrettoad closed this Nov 30, 2024
@secrettoad secrettoad reopened this Nov 30, 2024
@secrettoad
Copy link
Contributor Author

PyTorch and Tensorflow equivalence test that failed on previous build seems to be non-deterministic, or at the least not reproducible or related to this change. Passing now

@Rocketknight1
Copy link
Member

cc @gante @zucchini-nlp

@zucchini-nlp
Copy link
Member

For beam search I'd recommend to wait for gante as he had some plans for general refactoring, he'll be back later next week.

@secrettoad
Copy link
Contributor Author

@gante this seems like an easy fix to the bug mentioned. what do you think?

@gante
Copy link
Contributor

gante commented Jan 30, 2025

This is actually not the fix -- we might hit the stopping criteria with one of the batch items, but the other batch items would not be finished. The change would also break the case when early_stopping=False

Have a look at #34843 for related discussion, and at #35802 for the proper fix :)

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.

When set num_beams in GenerationConfig, stop_strings parameter has no effect

4 participants