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

fix: Make Recipe.model_dump() output compatible with model_validate() #1328

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ved1beta
Copy link

@ved1beta ved1beta commented Apr 6, 2025

SUMMARY:
Fixed issue #1319 where Recipe.model_dump() output couldn't be used with Recipe.model_validate(). Implemented an override of the model_dump() method to ensure it produces output in the format expected by validation, enabling proper round-trip serialization using standard Pydantic methods.

TEST PLAN:
Created test cases to verify fix works with both simple and complex recipes
Confirmed Recipe.model_validate(recipe.model_dump()) succeeds with various recipe formats
Validated that recipes with multiple stages having the same group name serialize/deserialize correctly
Ensured existing YAML serialization pathways continue to work as expected

@brian-dellabetta brian-dellabetta self-requested a review April 10, 2025 14:13
Copy link
Collaborator

@brian-dellabetta brian-dellabetta left a comment

Choose a reason for hiding this comment

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

Hi @ved1beta , thanks for the contribution! We should add some unit tests to make sure the recipes you checked manually get added to our CI tests. I can do this tomorrow, or if you have them handy still please add them to tests/llmcompressor/recipe/test_recipe.py

Copy link
Collaborator

@kylesayrs kylesayrs left a comment

Choose a reason for hiding this comment

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

Looks good! Nice job

@dsikka dsikka added the ready When a PR is ready for review label Apr 12, 2025
@dsikka dsikka added ready When a PR is ready for review and removed ready When a PR is ready for review labels Apr 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready When a PR is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants