Conversation
harshitgupta412
left a comment
There was a problem hiding this comment.
instead of checking whether prompt_strategy is None, it will be better to just define a DEFAULT_PROMPT_STRATEGY (there are already default args in PromptStrategy, so PromptStrategy() will be enough)
The default argument will then be PromptStrategy() instead of None
| user_instruction: str, | ||
| prompt_strategy: PromptStrategy, | ||
| operation_type: str = "filter", | ||
| ) -> tuple[list[dict[str, Any]], list[Any], list[str] | None]: |
There was a problem hiding this comment.
If I understand correctly, this is supposed to basically generate demonstrations using the teacher model for the given data. In that case, it will be better to use sem_ops directly with teacher model instead of bootstrap functions
There was a problem hiding this comment.
this will make sure that things are reusable and prevent double work in case some parameters change. (like adding a new prompt strategy or new prompts or new sem op)
Purpose
Refactors Reasoning Strategies to be more intuitive. Fixes #198 . Additionally, it fixes some of the inconsistencies with
sem_extractparticularly its output parsingTest Plan
Created comprehensive tests to test Reasoning with different operators and a mix of reasoning strategies.
Test Results
(Optional) Documentation Update
TODO: update docs on reasoning strategy
Type of Change
--> change to reasoning api
Checklist