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: assertion and chain-of-thought #995

Merged
merged 2 commits into from
May 9, 2024
Merged

fix: assertion and chain-of-thought #995

merged 2 commits into from
May 9, 2024

Conversation

Shangyint
Copy link
Collaborator

Previously, ChainOfThought does not work well with assertions due to an error in handling the rationale field of the CoT module. This pr fixes this issue. A future improvement would be implementing unit tests for CoT and Retry in test_retry.py.

@okhat
Copy link
Collaborator

okhat commented May 8, 2024

Thank you @Shangyint !! Looks like we had tests that are tied to the old behavior. Do we trmove/efit them?

@Shangyint
Copy link
Collaborator Author

Yes, I figured out the same thing. Pushed a fix!

@Shangyint Shangyint merged commit 339cb60 into main May 9, 2024
4 checks passed
@chiragshah285
Copy link

hey all @Shangyint the assertion fix broke a few things. I'm unable to use dspy.Suggest(), here is a minimal working example of an error: AttributeError: 'Predict' object has no attribute 'new_signature'. Did you mean: 'signature'?

https://gist.github.com/chiragshah285/cc1756cde35b8cf14aa04282f3f744e1

line 260 in assertions.py, i've patched it with this but not sure if this is the right fix:

                            if hasattr(error_state[0], 'new_signature'):
                                output_fields = error_state[0].new_signature.output_fields
                            else:
                                output_fields = error_state[0].signature.output_fields

@Shangyint Shangyint deleted the fix-assertions branch August 7, 2024 20:44
@Shangyint
Copy link
Collaborator Author

Hey @chiragshah285, thank you! I submitted your patch as #1372.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants