-
Notifications
You must be signed in to change notification settings - Fork 49
feat: composable requirements #91
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -156,6 +156,13 @@ def format_for_llm(self) -> TemplateRepresentation | str: | |
| template_order=["*", "Requirement"], | ||
| ) | ||
|
|
||
| def __and__(self, other): | ||
| return ConjunctiveRequirement([self,other]) | ||
| def __or__(self, other): | ||
| return DisjunctiveRequirement([self,other]) | ||
| def __not__(self): | ||
| return NegativeRequirement(self) | ||
|
|
||
|
|
||
| class LLMaJRequirement(Requirement): | ||
| """A requirement that always uses LLM-as-a-Judge. Any available constraint ALoRA will be ignored.""" | ||
|
|
@@ -178,6 +185,98 @@ def __init__(self, description: str, alora: Alora | None = None): | |
| self.alora = alora | ||
|
|
||
|
|
||
| class ConjunctiveRequirement(Requirement): | ||
| def __init__(self, requirements: list[Requirement],): | ||
| self.requirements = requirements | ||
|
|
||
| @property | ||
| def description(self): | ||
| return "\n* ".join( | ||
| ["Satisfy all of these requirements:"] + \ | ||
| [r.description for r in self.requirements]) | ||
|
|
||
| def validate(self, *args, **kwargs): | ||
| results = [r.validate(*args, **kwargs) for r in self.requirements] | ||
| return ValidationResult( | ||
| result = all(results), | ||
| reason = "\n* ".join( | ||
| ["These requirements are not satisfied:"]+ | ||
| [r.reason for r in results if not r]), | ||
| score = max([r.score for r in results if not r])) | ||
|
Comment on lines
+188
to
+205
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the rest of Mellea, a list of requirements is implicitly conjunctive. So does it make sense to have a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. List as conjunction by default is a good user interface but I believe internally they should be immediately converted into a conjunction. In a much more complex industrial use case where a company has a large set of rules, requirement as a list becomes unmanageable. I.e., compiling a NNF into CNF causes an exponential increase in the number of clauses. |
||
|
|
||
| def __and__(self, other): | ||
| match other: | ||
| case ConjunctiveRequirement(): | ||
| ConjunctiveRequirement(self.requirements+other.requirements) | ||
| case Requirement(): | ||
| ConjunctiveRequirement(self.requirements+[other]) | ||
|
|
||
| def __or__(self, other): | ||
| return DisjunctiveRequirement([self,other]) | ||
| def __not__(self): | ||
| return NegativeRequirement(self) | ||
|
|
||
|
|
||
|
|
||
| class DisjunctiveRequirement(Requirement): | ||
| def __init__(self, requirements: list[Requirement],): | ||
| self.requirements = requirements | ||
|
|
||
| @property | ||
| def description(self): | ||
| return "\n* ".join( | ||
| ["Satisfy at least one of these requirements:"] + \ | ||
| [r.description for r in self.requirements]) | ||
|
|
||
| def validate(self, *args, **kwargs): | ||
| results = [r.validate(*args, **kwargs) for r in self.requirements] | ||
| return ValidationResult( | ||
| result = any(results), | ||
| reason = "\n* ".join( | ||
| ["These requirements are satisfied:"]+ | ||
| [r.reason for r in results if r]), | ||
| score = min([r.score for r in results if not r])) | ||
|
|
||
| def __and__(self, other): | ||
| return ConjunctiveRequirement([self,other]) | ||
| def __or__(self, other): | ||
| match other: | ||
| case DisjunctiveRequirement(): | ||
| DisjunctiveRequirement(self.requirements+other.requirements) | ||
| case Requirement(): | ||
| DisjunctiveRequirement(self.requirements+[other]) | ||
| def __not__(self): | ||
| return NegativeRequirement(self) | ||
|
|
||
|
|
||
| class NegativeRequirement(Requirement): | ||
| def __init__(self, requirement: Requirement,): | ||
| self.requirement = requirement | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is kind of a weird thing to prompt with. But it's hard to know what a reasonable prompt would be here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We discussed this earlier offline but just for the historical note:
Xor expands to |
||
| @property | ||
| def description(self): | ||
| return f"Do not satisfy this requirement: {self.requirement.description}" | ||
|
|
||
| def __getattr__(self, name): | ||
| # delegate lookup to self.requirement | ||
| return getattr(self.requirement, name) | ||
|
|
||
| def validate(self, *args, **kwargs): | ||
| result = self.requirement.validate(*args, **kwargs) | ||
| return ValidationResult( | ||
| result = not result, | ||
| reason = result.reason, | ||
| # score = ??? | ||
| ) | ||
|
|
||
| def __and__(self, other): | ||
| return ConjunctiveRequirement([self,other]) | ||
| def __or__(self, other): | ||
| return DisjunctiveRequirement([self,other]) | ||
| def __not__(self): | ||
| return self.requirement | ||
|
|
||
|
|
||
| def reqify(r: str | Requirement) -> Requirement: | ||
| """Maps strings to Requirements. | ||
|
|
||
|
|
||
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.
descriptionis NOT a@propertyon the base class or in other Requirement types. Maybe it should be, but we should do this the same everywhere. Using property here but not in the other Requirement classes seems like a recipe for confusion down the road.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.
description is clearly a property in the base class. wdym?
Uh oh!
There was an error while loading. Please reload this page.
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 just mean that if we're going to use
property()to handleRequirement'sdescription, then we should do so consistently through-out the code base.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.
the use of property() here is explicitly to avoid insertion, which causes an inconsistency between the parent and the child requirement.