New advanced example: CPMpyXplain algorithm #253
Conversation
|
Thanks for the contribution; I confirm it runs fine. We will wait till after the project deadline to do a detailed review or changes, but it already looks solid as is. |
There was a problem hiding this comment.
Nice demonstration of the QuickXplain algorithm!
The logic looks good for the most part but the style of the example can use some love, I left some comments on what could be good to change before we merge it into master.
I really like the "iteration mode" as indeed the original paper relies heavily on the (actually arbitrary) ordering of the constraints...
When I use the "iteration mode", I get multiple of the same explanations, is that considered correct? Maybe you can check if the explanation is already found and not print it again?
Lastly: we have added a .set_description() option for constraints, maybe you can use it to nicely print the constraint relaxations here?
| life = intvar(0, 1100) | ||
| price = intvar(0, 300000) | ||
|
|
||
| global size_cons |
There was a problem hiding this comment.
Please do not use global variables, use them as arguments to your functions!
| import itertools | ||
|
|
||
| ## This variable is responsible for the use cases | ||
| enable_iteration = False |
There was a problem hiding this comment.
Better name could be "enumeration" imho
| - Default: One explanation, the order in which the constraints are considered is the same order as in which the user_constraints are mentioned in the 'user_constraints'-variable | ||
| - An explanation for every possible order in which the constraints can be considered | ||
| """ | ||
| from cpmpy import * |
There was a problem hiding this comment.
Use import cpmpy as cp instead
| Format: [Size, Memory, Life, Price] | ||
| All numbers must be multiplied by 100 since CPMpy can only work with Integer values. When creating the explanations, the values are automatically divided by 100. | ||
| """ | ||
| laptops = [[1540, 102400, 220, 149999], # Unsatisfiable, relax life time to get a satisfiable result |
There was a problem hiding this comment.
Maybe overkill but you can also structure this using a Pandas DataFrame, it might clean up the code later as you can simply index the columns you want using the name of the attribute of the laptop
| [1400, 51200, 1000, 189999] | ||
| ] | ||
|
|
||
| size = intvar(0, 2000) |
There was a problem hiding this comment.
Also give the name of the variable
| order_indices = list(range(0, len(user_constraints))) | ||
| orders = list(itertools.permutations(order_indices)) | ||
|
|
||
| for nb_explanation, order in enumerate(orders): |
There was a problem hiding this comment.
Performance suggestion: itertools.permutations returns a generator which means the next item is only generated on demand. Hence, instead of materializing each element to a list like you do on line 90, you can simply iterate over the generator directly:
for nb_expl, order in enumerate(permutations(order_indices)):|
|
||
|
|
||
| def no_sufficient_relax_space(): | ||
| for space in relaxation_spaces: |
There was a problem hiding this comment.
return all(len(space) > 1 for space in relaxation_spaces))|
|
||
| def relax_problem(order): | ||
| # Create a new model step by step until we find a satisfiable result (bottom-up) | ||
| new_model = Model(Table([size, memory, life, price], laptops)) |
There was a problem hiding this comment.
Actually I would consider the Table constraint also to be a background constraint
|
|
||
| def remove_duplicates(indices): | ||
| """ | ||
| Removes the duplicates from the given list |
There was a problem hiding this comment.
Is it not easier converting to a set?
| return result | ||
|
|
||
|
|
||
| def relax_constraint(index): |
There was a problem hiding this comment.
If you pass as argument constraint and relaxation_spaces here, it will become much nicer : )
|
Are there any news about it? It has been open for long time with minimal requested changes |
No description provided.