fix num_assistant_tokens with heuristic schedule #28759

jmamou · 2024-01-29T14:46:35Z

What does this PR do?

We have defined 2 different num_assistant_tokens_schedule values:

heuristic: When all speculative tokens are correct, increase num_assistant_tokens by 2 else reduce by 1. num_assistant_tokens value is persistent over multiple generation calls with the same assistant model.
heuristic_transient: Same as "heuristic but num_assistant_tokens is reset to its initial value after each generation call.

Fixes # (issue)
#27979 (comment)

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?

@gante @amyeroberts

gante

Thank you for the fix and the tests 💛

src/transformers/generation/configuration_utils.py

src/transformers/generation/candidate_generator.py

src/transformers/generation/utils.py

Co-authored-by: Joao Gante <[email protected]>

check that candidate_generator.assistant_model exists since some some speculations (like ngram and PLD) don't have assistant_model attribute

ArthurZucker

Thanks for the PR!

src/transformers/generation/candidate_generator.py

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

amyeroberts

Thanks for adding this!

For the code quality checks, you'll need to run make fixup and push the changes.

From the docstring, it mentions behaviour about `"heuristic_transient" being reset, but I don't see logic relating to it in this diff. Does this already happen?

tests/generation/test_utils.py

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

…transformers into fix_num_assistant_tokens

jmamou · 2024-02-08T12:58:49Z

Thanks for adding this!

For the code quality checks, you'll need to run make fixup and push the changes.

From the docstring, it mentions behaviour about `"heuristic_transient" being reset, but I don't see logic relating to it in this diff. Does this already happen?

I just fixed docstring

amyeroberts · 2024-02-12T12:28:32Z

@jmamou Let's try and make the CI green :) You'll need to resolve the quality checks by running make fixup and pushing the changes.

For the other failing tests, you'll need to try rebasing on main.

jmamou · 2024-02-12T14:39:38Z

@amyeroberts
tests_torch failed.
Are you familiar with the error?

amyeroberts · 2024-02-14T13:52:59Z

@jmamou Tbh, I'm not sure what the cause of these failures are. I would first suggest rebasing on main to make sure you have all of the most recent commits. This will trigger a re-run of the CI too.

jmamou · 2024-02-14T17:24:08Z

@amyeroberts
same test still fails 👎

gante · 2024-02-15T12:47:22Z

@jmamou Our apologies, this PR fixed it. Could you try rebasing again please? 🤗

gante · 2024-02-15T14:40:13Z

@amyeroberts both CI failures seem unrelated to this PR, including check_repository_consistency 👀 do you have an idea of what might be causing it?

amyeroberts · 2024-02-15T20:44:28Z

@jmamou Apologies for all of the current issues you've been experiencing with unrelated failures on this PR.

The two current batches of failing tests should have been resolved with #29037, #29043

Could you try one (final 🤞) rebase to get this CI green 🟢 ?

jmamou · 2024-02-16T09:46:53Z

@amyeroberts
tests passed on CI 👍

amyeroberts · 2024-02-16T11:45:24Z

@jmamou Thanks for this contribution and your patience with our misbehaving CI!

HuggingFaceDocBuilderDev · 2024-02-16T12:05:07Z

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

* fix heuristic num_assistant_tokens_schedule * Update src/transformers/generation/configuration_utils.py Co-authored-by: Joao Gante <[email protected]> * Update src/transformers/generation/candidate_generator.py Co-authored-by: Joao Gante <[email protected]> * Update utils.py check that candidate_generator.assistant_model exists since some some speculations (like ngram and PLD) don't have assistant_model attribute * Update src/transformers/generation/candidate_generator.py Co-authored-by: Arthur <[email protected]> * Update tests/generation/test_utils.py Co-authored-by: amyeroberts <[email protected]> * make fixup * merge conflict * fix docstring * make fixup --------- Co-authored-by: Joao Gante <[email protected]> Co-authored-by: Arthur <[email protected]> Co-authored-by: amyeroberts <[email protected]>

fix heuristic num_assistant_tokens_schedule

63c5f79

ArthurZucker requested a review from gante January 30, 2024 09:26

gante approved these changes Feb 7, 2024

View reviewed changes

src/transformers/generation/configuration_utils.py Outdated Show resolved Hide resolved

src/transformers/generation/candidate_generator.py Outdated Show resolved Hide resolved

gante reviewed Feb 7, 2024

View reviewed changes

src/transformers/generation/utils.py Outdated Show resolved Hide resolved

gante requested a review from amyeroberts February 7, 2024 16:34

jmamou and others added 3 commits February 7, 2024 23:06

Update src/transformers/generation/configuration_utils.py

13275fc

Co-authored-by: Joao Gante <[email protected]>

Update src/transformers/generation/candidate_generator.py

d0f84cc

Co-authored-by: Joao Gante <[email protected]>

Update utils.py

58b9819

check that candidate_generator.assistant_model exists since some some speculations (like ngram and PLD) don't have assistant_model attribute

ArthurZucker reviewed Feb 8, 2024

View reviewed changes

src/transformers/generation/candidate_generator.py Outdated Show resolved Hide resolved

Update src/transformers/generation/candidate_generator.py

acdb235

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

amyeroberts reviewed Feb 8, 2024

View reviewed changes

tests/generation/test_utils.py Outdated Show resolved Hide resolved

tests/generation/test_utils.py Outdated Show resolved Hide resolved

jmamou and others added 5 commits February 8, 2024 14:37

Update tests/generation/test_utils.py

f7f2f4d

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

make fixup

a248f1c

merge conflict

ec79ba6

Merge branch 'fix_num_assistant_tokens' of https://github.com/jmamou/…

bfaccc0

…transformers into fix_num_assistant_tokens

fix docstring

52cde6b

jmamou added 2 commits February 12, 2024 04:42

make fixup

04442ca

Merge branch 'huggingface:main' into fix_num_assistant_tokens

c9fbf13

Merge branch 'huggingface:main' into fix_num_assistant_tokens

1c09153

jmamou added 2 commits February 14, 2024 16:06

Merge branch 'huggingface:main' into fix_num_assistant_tokens

7b65e81

Merge branch 'huggingface:main' into fix_num_assistant_tokens

0906c49

Merge branch 'huggingface:main' into fix_num_assistant_tokens

69ed88e

Merge branch 'huggingface:main' into fix_num_assistant_tokens

3993ed3

amyeroberts merged commit 258da40 into huggingface:main Feb 16, 2024
21 checks passed

jmamou deleted the fix_num_assistant_tokens branch February 18, 2024 12:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix num_assistant_tokens with heuristic schedule #28759

fix num_assistant_tokens with heuristic schedule #28759

jmamou commented Jan 29, 2024

gante left a comment

ArthurZucker left a comment

amyeroberts left a comment

jmamou commented Feb 8, 2024

amyeroberts commented Feb 12, 2024

jmamou commented Feb 12, 2024

amyeroberts commented Feb 14, 2024

jmamou commented Feb 14, 2024

gante commented Feb 15, 2024

gante commented Feb 15, 2024

amyeroberts commented Feb 15, 2024

jmamou commented Feb 16, 2024

amyeroberts commented Feb 16, 2024

HuggingFaceDocBuilderDev commented Feb 16, 2024

fix num_assistant_tokens with heuristic schedule #28759

fix num_assistant_tokens with heuristic schedule #28759

Conversation

jmamou commented Jan 29, 2024

What does this PR do?

Before submitting

Who can review?

gante left a comment

Choose a reason for hiding this comment

ArthurZucker left a comment

Choose a reason for hiding this comment

amyeroberts left a comment

Choose a reason for hiding this comment

jmamou commented Feb 8, 2024

amyeroberts commented Feb 12, 2024

jmamou commented Feb 12, 2024

amyeroberts commented Feb 14, 2024

jmamou commented Feb 14, 2024

gante commented Feb 15, 2024

gante commented Feb 15, 2024

amyeroberts commented Feb 15, 2024

jmamou commented Feb 16, 2024

amyeroberts commented Feb 16, 2024

HuggingFaceDocBuilderDev commented Feb 16, 2024