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

Download fix #1366

Merged
merged 3 commits into from
Nov 19, 2024
Merged

Download fix #1366

merged 3 commits into from
Nov 19, 2024

Conversation

gabe-l-hart
Copy link
Contributor

Closes #1360
Replaces #1346

This is an updated fix for the root cause of the failure to download Mistral that was introduced in #1255

Copy link

pytorch-bot bot commented Nov 12, 2024

🔗 Helpful Links

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

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

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

✅ No Failures

As of commit 5747a71 with merge base 5da240a (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 Nov 12, 2024
@gabe-l-hart gabe-l-hart force-pushed the download-fix branch 2 times, most recently from a74ddc9 to 0c14af5 Compare November 12, 2024 19:48
leseb and others added 3 commits November 18, 2024 08:43
Downloading a Mistral model fails because it includes multiple weight
mapping files. The regression was introduced in commit
`766bee9f4a1fcb187fae543a525495d3ff482097`. I'm unclear on the original
intent, but perhaps the exception was meant to apply only to Granite
models. This isn’t an ideal fix, but it does enable Mistral to be
downloaded and used for chat.

Signed-off-by: Sébastien Han <[email protected]>
The previous logic didn't handle .bin files, so if a model (like mistral)
has both .bin and .safetensors, it would download both.

Branch: download-fix

Signed-off-by: Gabe Goodhart <[email protected]>
This will not actually be needed for mistral with the fix in download to
handle .bin files, but it may be needed for other models, so it's worth
having.

Branch: download-fix

Signed-off-by: Gabe Goodhart <[email protected]>
Copy link
Contributor

@Jack-Khuu Jack-Khuu left a comment

Choose a reason for hiding this comment

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

Looks solid to me, thanks for the follow up

@Gasoonjia @vmpuri Can one of yall sanity test then ship it?

@byjlw byjlw merged commit 57dee04 into pytorch:main Nov 19, 2024
52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.

AssertionError: Found multiple weight mapping files
7 participants