Skip to content

LlamaAttention forward function type hint is incorrect #38739 #38795

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from

Conversation

ArkVex
Copy link

@ArkVex ArkVex commented Jun 12, 2025

Hi, this PR fixes a small issue in the LlamaAttention class. The return type in the forward method currently shows three values, but the function actually returns only two. This seems to have been missed during the attention refactor (possibly in PR #35235).

I’ve updated the type hint to reflect the actual return values, just to avoid confusion for anyone reading or using the code. Let me know if any other changes are needed. Happy to help!

@Rocketknight1
Copy link
Member

Hi @ArkVex can you run make fix-copies to propagate that change to other models that are copying from Llama? That should make the CI pass!

@ArkVex
Copy link
Author

ArkVex commented Jun 13, 2025

Hi @ArkVex can you run make fix-copies to propagate that change to other models that are copying from Llama? That should make the CI pass!

I didnt get that...could you plz explain?

@Rocketknight1
Copy link
Member

Hi @ArkVex, if you look at the tests on this PR, "check_repository_consistency" is failing. The reason is that some other models copy from Llama, and those copies don't match after this PR. You should run make fix-copies in the transformers directory and then commit + push those changes, which should fix the problem.

@ArkVex
Copy link
Author

ArkVex commented Jun 13, 2025

Hi @ArkVex, if you look at the tests on this PR, "check_repository_consistency" is failing. The reason is that some other models copy from Llama, and those copies don't match after this PR. You should run make fix-copies in the transformers directory and then commit + push those changes, which should fix the problem.

Thanks for pointing it out...i will do the same

@ArkVex
Copy link
Author

ArkVex commented Jun 13, 2025

image
I did the test...plz check

@ArkVex
Copy link
Author

ArkVex commented Jun 14, 2025

@Rocketknight1 you there?

@ArkVex
Copy link
Author

ArkVex commented Jun 16, 2025

@vanpelt @dxoigmn @tmm1 anyone plz review

@Rocketknight1
Copy link
Member

Seems like make fix-copies isn't working correctly, very odd! I can run it, one sec.

@Rocketknight1
Copy link
Member

Hi @ArkVex, the reason it's failing is that the PR is on your fork's main branch. This breaks things! I recommend:

  1. Close this PR
  2. Make a new branch in your fork
  3. Push the change there
  4. Run make fix-copies
  5. Open a PR from that branch and ping me

@ArkVex
Copy link
Author

ArkVex commented Jun 16, 2025

Hi @ArkVex, the reason it's failing is that the PR is on your fork's main branch. This breaks things! I recommend:

  1. Close this PR
  2. Make a new branch in your fork
  3. Push the change there
  4. Run make fix-copies
  5. Open a PR from that branch and ping me

Okk oKk

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.

2 participants