Skip to content
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

Dead code, cache_kwargs #31763

Closed
4 tasks
freckletonj opened this issue Jul 3, 2024 · 3 comments
Closed
4 tasks

Dead code, cache_kwargs #31763

freckletonj opened this issue Jul 3, 2024 · 3 comments
Labels

Comments

@freckletonj
Copy link

System Info

This is rather low priority. I found it confusing chasing down the occurrences of cache_kwargs to try to understand how sin and cos are getting used in the Cache, presumably for RoPE, but it appears they are only used in the SinkCache which, as far as I can tell, isn't commonly made use of by any transformer. cache_positions appears to be used, but not sin and cos.

This dead bit of code shows up in 3 popular models, but also many others, and I don't think it has any bearing on anything at all:

Llama: https://github.com/huggingface/transformers/blob/main/src/transformers/models/llama/modeling_llama.py#L339
Mistral: https://github.com/huggingface/transformers/blob/main/src/transformers/models/mistral/modeling_mistral.py#L255
Qwen: https://github.com/huggingface/transformers/blob/main/src/transformers/models/qwen2/modeling_qwen2.py#L278

Per your tagging request: @ArthurZucker, and I saw @zucchini-nlp and @zhenglongjiepheonix in git-blame
And sorry if this is just noise, I'll shut up about small stuff if you like.

Who can help?

No response

Information

  • The official example scripts
  • My own modified scripts

Tasks

  • An officially supported task in the examples folder (such as GLUE/SQuAD, ...)
  • My own task or dataset (give details below)

Reproduction

nothing to reproduce, just dead code

Expected behavior

just dead code

@freckletonj
Copy link
Author

Ah, forgive me, it appears that these params are in place should someone choose to use the SinkCache, even if it is not the default.

If this is correct, maybe the issue is just one of documentation, but either way, please feel free to close this issue.

@ArthurZucker
Copy link
Collaborator

Yep this is correct, if you want to update the documentation feel free to do it!

@freckletonj
Copy link
Author

I appreciate the confirmation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants