Skip to content

Conversation

@jiqing-feng
Copy link
Contributor

@jiqing-feng jiqing-feng commented Dec 5, 2023

Hi @ArthurZucker @younesbelkada . Since _prepare_4d_attention_mask is no longer a member function of AttentionMaskConverter, I directly import _prepare_4d_attention_mask from modeling_attn_mask_utils. Would you please help to review it? Thx!

BTW, the failed CIs are not related to my changes

@jiqing-feng jiqing-feng changed the title use _prepare_4d_attention_mask Fix bug of _prepare_4d_attention_mask Dec 5, 2023
@jiqing-feng jiqing-feng marked this pull request as ready for review December 5, 2023 06:53
Copy link
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

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

The failing CI is related to the styling, make style should help

"Calling `transformers.models.llama.modeling_llama._prepare_4d_attention_mask` is deprecated and will be removed in v4.37. Use `transformers.modeling_attn_mask_utils.AttentionMaskConverter._prepare_4d_attention_mask"
)
return AttentionMaskConverter._prepare_4d_attention_mask(mask=mask, dtype=dtype, tgt_len=tgt_len)
return _prepare_4d_attention_mask(mask=mask, dtype=dtype, tgt_len=tgt_len)
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is deprecated anyway, but good catch.

Copy link
Contributor

@younesbelkada younesbelkada left a comment

Choose a reason for hiding this comment

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

Nice catch! I left one comment, for the failing CI can you try to merge with main and make sure you have ruff==0.1.5 installed when running make fixup ?

@jiqing-feng
Copy link
Contributor Author

Hi @ArthurZucker @younesbelkada . All CIs are green, would you please help me review and merge it? Thx!

Copy link
Contributor

@younesbelkada younesbelkada left a comment

Choose a reason for hiding this comment

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

Thanks!

@ArthurZucker ArthurZucker merged commit 4d806db into huggingface:main Dec 7, 2023
@jiqing-feng jiqing-feng deleted the llama branch January 10, 2024 05:47
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.

3 participants