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.

Sign up for GitHub

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

Jump to bottom

Update fusion layer counting logic for Llama 3.2 weight conversion #1722

Merged
merged 3 commits into from
Sep 30, 2024
Merged

Update fusion layer counting logic for Llama 3.2 weight conversion #1722

merged 3 commits into from
Sep 30, 2024

Conversation

ebsmothers
Copy link
Contributor

@ebsmothers ebsmothers commented Sep 30, 2024 •
edited
Loading

Checkpoint save errors without this change

Test plan:

tune run lora_finetune_single_device --config llama3_2_vision/11B_lora_single_device max_steps_per_epoch=1

Before: https://gist.github.com/ebsmothers/c9ad0175cedeb5ad2719aec4d266090d

After:

INFO:torchtune.utils._logging:Saving final epoch checkpoint.
INFO:torchtune.utils._logging:The full model checkpoint, including all weights and configurations, has been saved successfully.You can now use this checkpoint for further training or inference.
INFO:torchtune.utils._logging:Checkpoint saved in 46.25 seconds.

@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented Sep 30, 2024 •
edited
Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/torchtune/1722

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

✅ No Failures

As of commit df8cab3 with merge base 3fddc56 (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 Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 30, 2024
RdoubleA
RdoubleA reviewed Sep 30, 2024
View reviewed changes
torchtune/models/llama3_2_vision/_convert_weights.py
num_fusion_layers = (
max(_layer_num(k) for k in state_dict if "cross_attention_layers" in k) + 1
num_fusion_layers = len(
set([k.split(".")[2] for k in state_dict if "fusion_layer" in k])
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add a comment explaining what the FQN looks like here? Why not just count number of "fusion_layer" in k for k in state_dict?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why not just count number of "fusion_layer" in k for k in state_dict?

There are multiple params for each layer so we need to dedup

@felipemello1
Copy link
Contributor

related: #1721

pbontrager
pbontrager reviewed Sep 30, 2024
View reviewed changes
torchtune/models/llama3_2_vision/_convert_weights.py
@@ -148,8 +148,8 @@ def llama3_vision_tune_to_meta(

# Calculate fusion_interval: layer interval where cross attention layers are fused
num_layers = max(_layer_num(k) for k in state_dict if "layers" in k) + 1
num_fusion_layers = (
max(_layer_num(k) for k in state_dict if "cross_attention_layers" in k) + 1
num_fusion_layers = len(
Copy link
Contributor

@pbontrager pbontrager Sep 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

What is the [2] referring to here? Isn't that the layer number?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah exactly

pbontrager
pbontrager approved these changes Sep 30, 2024
View reviewed changes
Copy link
Contributor

@pbontrager pbontrager left a comment

Choose a reason for hiding this comment

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

Thanks for catching this. Can you just add a comment above the change saying you're getting the unique layer numbers or use the _layer_number function?

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 67.64%. Comparing base (6bc143f) to head (df8cab3).
Report is 20 commits behind head on main.

Files with missing lines Patch % Lines
...rchtune/models/llama3_2_vision/_convert_weights.py 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1722      +/-   ##
==========================================
- Coverage   70.67%   67.64%   -3.03%     
==========================================
  Files         299      304       +5     
  Lines       15251    15627     +376     
==========================================
- Hits        10778    10571     -207     
- Misses       4473     5056     +583
Flag Coverage Δ
67.64% <0.00%> (-3.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ebsmothers ebsmothers merged commit 10b02e0 into pytorch:main Sep 30, 2024
17 checks passed
@ebsmothers ebsmothers deleted the llama3_2_vision_fix branch September 30, 2024 22:19
@c2huc2hu c2huc2hu mentioned this pull request Sep 30, 2024
Closed
13 tasks
RdoubleA pushed a commit that referenced this pull request Oct 2, 2024
@ebsmothers @RdoubleA
Update fusion layer counting logic for Llama 3.2 weight conversion (#…
ff2a85f 
…1722)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RdoubleA RdoubleA RdoubleA left review comments

@pbontrager pbontrager pbontrager approved these changes

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@ebsmothers @felipemello1 @codecov-commenter @pbontrager @RdoubleA @facebook-github-bot