Leaking rows on failed addConstraint about kiwi HOT 7 CLOSED

I believe @sccolbert was not worried about that at the time of writing (cf comment)

                // Creating a row causes symbols to reserved for the variables
		// in the constraint. If this method exits with an exception,
		// then its possible those variables will linger in the var map.
		// Since its likely that those variables will be used in other
		// constraints and since exceptional conditions are uncommon,
		// i'm not too worried about aggressive cleanup of the var map.
		Tag tag;
		std::auto_ptr<Row> rowptr( createRow( constraint, tag ) );
                Symbol subject( chooseSubject( *rowptr, tag ) );

from kiwi.

Well, it is a major problem. We cache the constraints for the lifetime of the system, which means we assume unsatisfiable constraint exceptions will leave the solver in a consistent state (i.e. as if not added at all). The problem is when we add seemingly unrelated constraints later on, they will trigger spurious UnsatisfiableConstraint exceptions as there are some ghost rows in the solver. The fix seems to be easy as well:

            // If the artificial variable is basic, pivot the row so that
	// it becomes basic. If the row is constant, exit early.
	RowMap::iterator it = m_rows.find( art );
	if( it != m_rows.end() )
	{
        std::auto_ptr<Row> rowptr(it->second);
        m_rows.erase(it);
        if (rowptr->cells().empty())
            return success;

        Symbol entering(anyPivotableSymbol(*rowptr));
        if (entering.type() == Symbol::Invalid)
            return false;  // unsatisfiable (will this ever happen?)

        if (success)
        {
            rowptr->solveFor(art, entering);
            substitute(entering, *rowptr);
            m_rows[entering] = rowptr.release();
        }
	}

from kiwi.

Could you make a PR and add a test ideally ?

from kiwi.

ping @sccolbert

from kiwi.

IIRC (and my memory is hazy here) the problem of removing the constraint can be difficult because the tableau may have been pivoted before it can be known that the constraint is unsatisfiable. There is no logic for reversing a pivot and the parts of the constraint may be distributed across multiple rows.

from kiwi.

If you have an unsatisfiable constraint, the easiest thing to do is just create a new solver and add the known-good constraints.

from kiwi.

Thanks @sccolbert for your feedback. @bucurb if you can provide a PR (with a test ideally) I will take time to investigate in more details and review it carefully but otherwise I probably won't look at it for some time.

from kiwi.