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

Fixed Gemma2 error when saving pretrain #1781

Merged
merged 1 commit into from
Feb 14, 2025

Conversation

kplau1128
Copy link
Contributor

Fixed Gemma2 model error when saving pretrain.
Due to model.config has non-serializable type.

@kplau1128 kplau1128 requested a review from regisss as a code owner February 13, 2025 06:54
@kplau1128 kplau1128 changed the title Fixed Gemma2 save pretrain error Fixed Gemma2 error when saving pretrain Feb 13, 2025
@regisss
Copy link
Collaborator

regisss commented Feb 13, 2025

Not sure why this was added for Gemma2, I thought we only enabled it for Llama 🤔
Any idea @Luca-Calabria @kalyanjk ?

@kplau1128
Copy link
Contributor Author

This paralllel_strategy was just initialized but it never been use in Gemma2. And it is not JSON serializable.

@Luca-Calabria
Copy link
Contributor

@regisss unfortunately Gemma2 enabling was started by another guy and I closed it after months in standby. Can be possible that this parallel_strategy was added wrongly during the first enabling pass but not fully validated.

@libinta libinta added the run-test Run CI for PRs from external contributors label Feb 14, 2025
@kplau1128
Copy link
Contributor Author

verified text-gen examples with google/gemma-2-9b-it model on 1x/multi-x HPU both passed, no regression as expected

@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Copy link
Collaborator

@regisss regisss left a comment

Choose a reason for hiding this comment

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

LGTM!

@regisss regisss merged commit 2704931 into huggingface:main Feb 14, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-test Run CI for PRs from external contributors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants