Skip to content
This repository was archived by the owner on Sep 10, 2025. It is now read-only.

Conversation

@Gasoonjia
Copy link
Contributor

We use a new rope class after bumped tune version.

@pytorch-bot
Copy link

pytorch-bot bot commented Sep 25, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/torchchat/1202

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 078ac23 with merge base 021fd32 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Sep 25, 2024

from torchchat.model import Model, ModelArgs, ModelType

from torchtune.modules.position_embeddings import RotaryPositionalEmbeddings
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's get rid of the old one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

for submodule in model.modules():
if isinstance(submodule, RotaryPositionalEmbeddings):
if isinstance(submodule, Llama3ScaledRoPE):
submodule.__init__(head_dim, max_seq_len, rope_base)
Copy link
Contributor

@Jack-Khuu Jack-Khuu Sep 25, 2024

Choose a reason for hiding this comment

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

nit: We should be populating the rest of the RoPE fields based on the config. This can be a separate PR

From tune:

scale_factor: int = 8,
low_freq_factor: int = 1,
high_freq_factor: int = 4,
old_context_len: int = 8192,

@Jack-Khuu Jack-Khuu changed the title update rope class Update Flamingo Builder to use Llama3ScaledRoPE instead of RotaryPositionalEmbeddings Sep 25, 2024
@Gasoonjia Gasoonjia merged commit 0b8ca05 into main Sep 25, 2024
51 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants