adding QuickXplain #416
adding QuickXplain #416
Conversation
|
Added quickxplain also to the tools and implemented a naive version. |
| from cpmpy.expressions.variables import NDVarArray | ||
| from cpmpy.transformations.get_variables import get_variables | ||
| from cpmpy.transformations.normalize import toplevel_list | ||
|
|
||
|
|
||
| def mus(soft, hard=[], solver="ortools"): | ||
| """ |
There was a problem hiding this comment.
If you are proposing improvements to MUS as well, then I would rename variable solver to solvername.
| m = Model(hard+[assump.implies(candidates)]) # each assumption variable implies a candidate | ||
| s = SolverLookup.get(solver, m) | ||
| m = cp.Model(hard + [assump.implies(candidates)]) # each assumption variable implies a candidate | ||
| s = cp.SolverLookup.get(solver, m) |
There was a problem hiding this comment.
Some thoughts for readability.
m -> model
s -> solver
There was a problem hiding this comment.
I'm only proposing changes for the quickXplain algo : )
But I do agree with your proposed improvements, so maybe we can put them in antoher PR?
| return [dmap[a] for a in core] | ||
|
|
||
|
|
||
| def recurse_explain(soft, hard, delta, solver): |
There was a problem hiding this comment.
Is there a reason why you recurse_explain (resp. do_recursion) in the tools is embedded and not here ?
Change the function name to be consistent across both files?
There was a problem hiding this comment.
Not really, I wanted to stay very close to the original pseudocode of the paper in the example, while the tools-version has some subtle optimizations, plus embedding the recursive call ensures users cannot call the helper function themselves by accident.
I implemented the original basic version of the QuickXplain algorithm of Junker: