-
Notifications
You must be signed in to change notification settings - Fork 54
feat: add requirements and preconditions to gen slots #226
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
Merge ProtectionsYour pull request matches the following merge protections and will not be merged until they are valid. 🟢 Enforce conventional commitWonderful, this rule succeeded.Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
| # Reqs to use with the strategy. You could also just remove "unknown" from the structured output for this. | ||
| requirements=["avoid classifying the sentiment as unknown"], | ||
| strategy=RejectionSamplingStrategy(), # Must specify a strategy for gen slots | ||
| ) |
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.
I was thinking of these parameters as parameters to the decorator. Was. there a reason to add it to the function call?
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, there were some python type hinting limitations that could not be easily overcome: #190 (comment)
| m, | ||
| text="I love this!", | ||
| # Preconditions are only checked with basic validation. Don't use the strategy. | ||
| precondition_requirements=["the text arg should be less than 100 words"], |
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.
That's cool. Is there a scenario where a precondition_check would be helpful for m.instruct as well?
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.
I think most of the functionality that we add to a function like @generative or m.instruct, can / should be exposed at the level of m.act at least. However, I think we would need to figure out all the hooks / processing / validation steps we want before doing that.
…ecorated func params as positional args
4cc52ad to
84f59f6
Compare
|
failing test wasn't caused by generative slots; everything else looked good |
Generative slot tests and examples all passed / ran.
Closes #190 and #216
Features:
def test((one: int, two: bool, three:str) -> int:)previous:
Arguments:... - three: (type: <class 'str'>)now:
Arguments:... - three: "" (type: <class 'str'>)Semi-related changes and open questions:
outputparameter.Examples of the templated output: