-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Refactor finetuning implementation to be 2.5 compatible #1594
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
Conversation
chenmoneygithub
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
Agreed with @okhat, the PR can use some refactoring to have the finetuning code in a standalone mode.
dspy/predict/predict.py
Outdated
| def assert_structural_equivalency_for_predictors( | ||
| predictor1: object, | ||
| predictor2: object, | ||
| ) -> Optional[AssertionError]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this type hint is a bit confusing, we can omit the type hints here.
chenmoneygithub
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update! Most comments are on nits.
|
Besides the things discussed on Slack, this is good to merge except for |
|
Incredible work on this @dilarasoylu & @isaacbmiller !! |
4eb4f09 to
347e1a4
Compare
347e1a4 to
1eb95ce
Compare
cc @dilarasoylu
relevant anysacle notebook is anyscale/templates#381