-
Notifications
You must be signed in to change notification settings - Fork 146
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
Set Expressions (with Chainable Format) for Arithmetic and Relational Operations #1107
base: develop
Are you sure you want to change the base?
Conversation
b2f0ce3
to
020ac80
Compare
Thank you for the PR. |
if (result == null) { | ||
switch (op) { | ||
case SUBSET: | ||
result = model.subsetEq(xSet, ySet).reify().eq(TRUE).boolVar(); |
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 expected expression is : (xSet \in ySet) \impl result
but you defined: ((xSet \in ySet) \impl True) \imp result
.
You can simplify the expression to:
result = model.subsetEq(xSet, ySet).reify();
|
||
import java.util.HashSet; | ||
|
||
import static org.chocosolver.solver.constraints.real.Ibex.TRUE; |
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.
Ibex.TRUE
is a constant for continuous constraint programming and should not be used in Set expressions
result = model.subsetEq(xSet, ySet).reify().eq(TRUE).boolVar(); | ||
break; | ||
case EQ: | ||
result = model.allEqual(xSet, ySet).reify().eq(TRUE).boolVar(); |
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.
same as l.55
result = model.allDifferent(xSet, ySet).reify().eq(TRUE).boolVar(); | ||
break; | ||
case CONTAINS: | ||
result = model.subsetEq(ySet, xSet).reify().eq(TRUE).boolVar(); |
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.
same as l.55
result = model.allEqual(xSet, ySet).reify().eq(TRUE).boolVar(); | ||
break; | ||
case NE: | ||
result = model.allDifferent(xSet, ySet).reify().eq(TRUE).boolVar(); |
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.
same as l.55
result = model.subsetEq(ySet, xSet).reify().eq(TRUE).boolVar(); | ||
break; | ||
case NC: | ||
result = model.disjoint(ySet, xSet).reify().eq(TRUE).boolVar(); |
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.
same as l.55
import org.chocosolver.solver.variables.SetVar; | ||
|
||
public interface SetExpression { | ||
default SetExpression union(SetExpression y){ |
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 suppose we can have JavaDoc here
|
||
@Override | ||
public void extractVar(HashSet<IntVar> variables) { | ||
} |
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 would throw an exception here, to be safe
|
||
private BoolVar result; | ||
|
||
public UnReSetExpression(SetOperator operator, SetExpression x, int... y) { |
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'm not sure to understand why this class is needed, everything can be done with BiReSetExpression
, no?
* setA.union(setB).post(); | ||
*/ | ||
|
||
public class SetVarExpression extends SetVarImpl implements SetExpression { |
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 don't think this class is useful, can't SetVar implement SetExpression directly?
Set Expressions (with Chainable Format) for Arithmetic and Relational Operations
I've been working with
SetVar
inchoco-Solver
. Unlike other types of variables, such asRealVar
orIntVar
, there was no direct way to compose operations such as equals (eq), not equals (ne), contains, not contains (nc), subset (subSet), union, or intersection directly in the model definition. While propagators allow for constructing such constraints, it was not as intuitive as it is for other variable types likeIntVar
andRealVar
. These features are extremely useful in facilitating the composition of constraints.Thus, I've worked on adding this feature for
SetVar
, based on the operations already available for other variable types. For example:However, such expressions were not possible for
SetVar
:Improvement
I've implemented a new expression framework for
SetVar
, allowing the use of familiar operations directly on set variables. This change introduces support for the following operations:Relational Operations:
eq
(Equals):setA.eq(1, 2, 3)
orsetA.eq(setB)
.ne
(Not Equals):setA.ne(1, 2)
orsetA.ne(setB)
.contains
:setA.contains(1, 2)
.nc
(notContains):setA.notContains(1, 2)
.subSet
:setA.subSet(1, 2, 3)
orsetA.subSet(setB)
.Arithmetic Set Operations:
union
:setA.union(setB)
intersection
:setA.intersection(setB)
This enhancement allows for the expression of complex set relations with simplified syntax, enabling combinations such as:
Example Usage:
Internal Changes:
This change is implemented through three main components:
UnReSetExpression
): Handle relations between a set and a list of values, likesetVar.eq(1,2,3).post()
.BiReSetExpression
): Manage operations between twoSetVar
instances, such assetA.eq(setB).post()
.BiArSetExpression
): Handle arithmetic operations like unions and intersections, e.g.,setA.eq(setB.union(setC)).post()
.A new interface,
SetExpression
, inspired by other Choco-Solver expressions, allowsSetVarExpression
to be used in conjunction with these expression composition objects (UnReSetExpression
,BiReSetExpression
, andBiArSetExpression
).Backward Compatibility:
This improvement is fully backward-compatible with existing constraints and models. It adds considerable flexibility when working with
SetVar
, and I believe it will make modeling with sets much more intuitive.Future Improvements:
I believe that additional operations, such as disjoint, could also be implemented to further enhance set operations in Choco-Solver.
Finally, I've included a comprehensive suite of tests in
SetVarExpressionTest
, which I hope will be useful to validate this enhancement.I would greatly appreciate any feedback or suggestions for optimizations, as well as any edge cases that could further improve this approach!