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

RoPE updates #412

Merged
merged 5 commits into from
Oct 23, 2024
Merged

RoPE updates #412

merged 5 commits into from
Oct 23, 2024

Conversation

rasbt
Copy link
Owner

@rasbt rasbt commented Oct 23, 2024

This PR addresses an issue with the RoPE frequency calculation (thanks @d-kleine) and input example dimensions (thanks @rkinas)

Fixes #410 and #411

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@rasbt rasbt merged commit 7cd6a67 into main Oct 23, 2024
8 checks passed
@rasbt rasbt deleted the rope-updates branch October 23, 2024 23:07
@d-kleine
Copy link
Contributor

@rasbt Do the pytests work for you? I get an error

tests.py:295: TypeError
=======short test summary info ========
FAILED tests.py::test_rope_llama3_12 - TypeError: LlamaRotaryEmbedding.init() got an unexpected keyword argument 'config'

@rasbt
Copy link
Owner Author

rasbt commented Oct 23, 2024

Hm weird, worked for me:

Screenshot 2024-10-23 at 6 40 27 PM

@rasbt
Copy link
Owner Author

rasbt commented Oct 23, 2024

Oh, could you perhaps upgrade the transformer installation?

@d-kleine
Copy link
Contributor

d-kleine commented Oct 23, 2024

Yes, worked, thanks 🙂✅
grafik

Could you please also add pytest to the test-requirements-extra.txt file?

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.

RoPE inv_freq code
2 participants