Skip to content

Fix cache-related tests#39676

Merged
zucchini-nlp merged 9 commits intohuggingface:mainfrom
zucchini-nlp:fix-ci-cache
Jul 28, 2025
Merged

Fix cache-related tests#39676
zucchini-nlp merged 9 commits intohuggingface:mainfrom
zucchini-nlp:fix-ci-cache

Conversation

@zucchini-nlp
Copy link
Copy Markdown
Member

@zucchini-nlp zucchini-nlp commented Jul 25, 2025

What does this PR do?

As per title, failing after latest cache compatibility PR

@zucchini-nlp
Copy link
Copy Markdown
Member Author

run-slow: kyutai_speech_to_text, musicgen_melody, qwen2_5_omni, rag, roformer, superglue

@github-actions
Copy link
Copy Markdown
Contributor

This comment contains run-slow, running the specified jobs:

models: ['models/kyutai_speech_to_text', 'models/musicgen_melody', 'models/qwen2_5_omni', 'models/rag', 'models/roformer', 'models/superglue']
quantizations: [] ...

@HuggingFaceDocBuilderDev
Copy link
Copy Markdown

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.

@zucchini-nlp zucchini-nlp requested a review from ydshieh July 28, 2025 09:52
@zucchini-nlp
Copy link
Copy Markdown
Member Author

run-slow: kyutai_speech_to_text, musicgen, musicgen_melody, rag, roformer, superglue

@github-actions
Copy link
Copy Markdown
Contributor

This comment contains run-slow, running the specified jobs:

models: ['models/kyutai_speech_to_text', 'models/musicgen', 'models/musicgen_melody', 'models/rag', 'models/roformer', 'models/superglue']
quantizations: [] ...

@github-actions
Copy link
Copy Markdown
Contributor

[For maintainers] Suggested jobs to run (before merge)

run-slow: kyutai_speech_to_text, llava_next, llava_next_video, musicgen, musicgen_melody, qwen2_5_omni, qwen2_5_vl, qwen2_vl, rag, roformer, superglue

Copy link
Copy Markdown
Collaborator

@ydshieh ydshieh left a comment

Choose a reason for hiding this comment

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

Not very familiar with some changes in src/transformers so I wll rely on CI and trust you. Just a few nit questions to get some idea.

Thank you!

Could you link the PR you mentioned in this PR description?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

do you know what causse this changes?

Copy link
Copy Markdown
Member Author

@zucchini-nlp zucchini-nlp Jul 28, 2025

Choose a reason for hiding this comment

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

yep, after #39374 we started using higher default max length. Text-only generation pipe already uses it, so it's fine

logits_padded = res_padded.logits[inputs_dict["attention_mask"].bool()]
logits_padfree = res_padfree.logits[0]

torch.testing.assert_close(logits_padded.argmax(-1), logits_padfree.argmax(-1), rtol=0, atol=0)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ok, can't find it on the common test file, so fine.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this one doesn't make sense because we want to check logits, not sampled argmax tokens. Even tiny diff in logits can give different tokens, and the next line check with torch.allclose is enough

Comment on lines +264 to +273
# Apply RoPE if self attention
if not is_cross_attention and sinusoidal_pos is not None:
if self.rotary_value:
query_layer, key_layer, value_layer = self.apply_rotary_position_embeddings(
sinusoidal_pos, query_layer, key_layer, value_layer
)
else:
query_layer, key_layer = self.apply_rotary_position_embeddings(
sinusoidal_pos, query_layer, key_layer
)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

was it deleted at some point and here you just add it back?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yep, accidentally deleted RoPE 🙈

@zucchini-nlp zucchini-nlp added the for patch Tag issues / labels that should be included in the next patch label Jul 28, 2025
@zucchini-nlp zucchini-nlp merged commit 1c6b474 into huggingface:main Jul 28, 2025
26 checks passed
ArthurZucker pushed a commit that referenced this pull request Jul 29, 2025
* fix

* fix kyutai at last

* fix unrelated tests and copies

* update musicgen as well

* revert tensor

* fix old test failures

* why it wasn't added?
zaristei pushed a commit to zaristei/transformers that referenced this pull request Sep 9, 2025
* fix

* fix kyutai at last

* fix unrelated tests and copies

* update musicgen as well

* revert tensor

* fix old test failures

* why it wasn't added?
zaristei pushed a commit to zaristei/transformers that referenced this pull request Sep 9, 2025
* fix

* fix kyutai at last

* fix unrelated tests and copies

* update musicgen as well

* revert tensor

* fix old test failures

* why it wasn't added?
zaristei pushed a commit to zaristei/transformers that referenced this pull request Sep 9, 2025
* fix

* fix kyutai at last

* fix unrelated tests and copies

* update musicgen as well

* revert tensor

* fix old test failures

* why it wasn't added?
zaristei pushed a commit to zaristei/transformers that referenced this pull request Sep 9, 2025
* fix

* fix kyutai at last

* fix unrelated tests and copies

* update musicgen as well

* revert tensor

* fix old test failures

* why it wasn't added?
zaristei pushed a commit to zaristei/transformers that referenced this pull request Sep 9, 2025
* fix

* fix kyutai at last

* fix unrelated tests and copies

* update musicgen as well

* revert tensor

* fix old test failures

* why it wasn't added?
zaristei pushed a commit to zaristei/transformers that referenced this pull request Sep 9, 2025
* fix

* fix kyutai at last

* fix unrelated tests and copies

* update musicgen as well

* revert tensor

* fix old test failures

* why it wasn't added?
zaristei pushed a commit to zaristei/transformers that referenced this pull request Sep 9, 2025
* fix

* fix kyutai at last

* fix unrelated tests and copies

* update musicgen as well

* revert tensor

* fix old test failures

* why it wasn't added?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

for patch Tag issues / labels that should be included in the next patch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants