Add Python API to get all constraints about kiwi HOT 14 OPEN

The main reason why this doesn't already exist, is because the solver doesn't actually store the Python constraint object under the hood. It stores the C++ constraint object which is created internally when you create a constraint from Python. So in order to return a list of Python constraints from a method like solver.constraints(), the solver would have to allocate new Python constraint objects to wrap the C++ constraint objects. That would introduce a few gotcha's when it comes to comparing objects via is.

If we were to add this, we would need to update the Python wrapper class to hold an internal list of the constraints, similar to what your sample code above is doing. We wouldn't make a change on the C++ implementation class.

from kiwi.

Doing this would require to also extend the C++ API. I know how to do it and I can see the value in the feature but I would like @sccolbert opinion on such a change, before working on it.

from kiwi.

@sccolbert Any thoughts on this?

from kiwi.

from kiwi.

Yes, but in my codebase constraints are added in a number of different places, it would be a messy and intrusive change to keep track of all of them myself just for debugging.

That's why if we could get access to the constraints from the solver, which already has a list of them, it would be much more convenient.

from kiwi.

from kiwi.

I think in the meantime that's what I'll do. But if anyone else needs this functionality in the future, or if I need it for a different project, we'll have to reimplement it in those projects. Since this library already has access to the list of constraints, it makes more sense to me to have this exposed from the library itself than have users reimplement this.

Is there any reason you're thinking of why we shouldn't do this?

from kiwi.

Related to this, other classes like UnsatisifiableConstraint could also expose the actual constraint object being violated. I'm currently catching the exception, parsing the message string for some variable names, and then searching through the list of constraints in my wrapper class for variables that have that matching name. It would be easier if I could just do e.constraint to get the constraint being violated.

from kiwi.

If anyone else needs this:

from typing import Optional

import kiwisolver
from kiwisolver import Constraint, UnsatisfiableConstraint, Variable

class Solver:
    def __init__(self):
        self._solver = kiwisolver.Solver()
        self._constraints = set()

    def _find(self, v: str) -> tuple[Optional[Variable], set[Constraint]]:
        res = set()
        res_strs = set()
        var = None
        for c in self._constraints:
            for term in c.expression().terms():
                if term.variable().name() == v:
                    var = term.variable()

                    # Skip duplicate constraints
                    if str(c) in res_strs:
                        continue

                    res_strs.add(str(c))
                    res.add(c)

        return var, res

    def add(self, c: Constraint) -> None:
        self._constraints.add(c)
        self._solver.addConstraint(c)

    def remove(self, c: Constraint) -> None:
        self._constraints.remove(c)
        self._solver.removeConstraint(c)

    def hasConstraint(self, c: Constraint) -> bool:
        return self._solver.hasConstraint(c)

    def update(self):
        self._solver.updateVariables()

    def analyze(
        self, e: UnsatisfiableConstraint
    ) -> list[tuple[Variable, set[Constraint]]]:
        variables = []

        msg = str(e)
        constraint, _ = msg.split(" | ")
        op = None
        for _op in [" <= ", " >= ", " == "]:
            if _op in constraint:
                op = _op
                break

        assert op is not None

        expr, constant = constraint.split(op)
        assert constant == "0"
        terms = expr.split(" + ")
        for term in terms:
            res = term.split(" * ")
            if len(res) == 2:
                _, var = res
                variables.append(self._find(var))

        return variables

Calling the analyze function with an UnsatisfiableConstraint exception produces output that's significantly more easy to debug than calling solver.dumps().

from kiwi.

I think this makes sense, actually - especially with bool violated() now being a thing. Being able to pass a Solver reference to a function and asking it "give me a list of constraints that failed" would be immensely helpful to have without needing to have an extra collection of constraints, thus duplicating memory in a lot of ways.

from kiwi.

@gsingh93 Regarding the UnsatisfiableConstraint. The constraint object should be contained in the .value attribute of the exception:

https://github.com/nucleic/kiwi/blob/main/py/src/solver.cpp#L59

from kiwi.

At least I always thought it was the .value attribute. Whoops. Turns out it's .args[0].

from kiwi.

So in order to return a list of Python constraints from a method like solver.constraints(), the solver would have to allocate new Python constraint objects to wrap the C++ constraint objects. That would introduce a few gotcha's when it comes to comparing objects via is.

Got it, thanks for the explanation. I think this could still be valuable if it's disabled by default and enabled with some flag that makes it clear it's only for debugging, but it's also fine to not include it and people can just something similar to the wrapper code I posted above. Alternatively, it would be nice if there was some optional argument to dumps where I could just dump constraints related to the variables I cared about, like in the wrapper code I posted above (the wrapper code only dumps constraints where the variable is directly in the constraint, which is already very helpful, but if this was implemented natively it may be easier to dump all constraints that affect the variable, even if the variable isn't directly in that particular constraint).

Regarding the UnsatisfiableConstraint. The constraint object should be contained in the .value attribute of the exception. At least I always thought it was the .value attribute. Whoops. Turns out it's .args[0].

Awesome, thanks, I'll switch to using this. Would be nice to have a dedicated attribute like .value or .constraint so it's easier to discover (I made an empty UnsatisifiableConstraint object and didn't see anything related in __dir__ so I figured it may not exist).

from kiwi.

from kiwi.