ConstraintFunction simplification

[aescande]

Is there some cases where ConstraintFunction here is templated by multiple classes. There are no such case in Tasks and I could find one in client code either.

Getting rid of the variadic template gives a much simpler code:

template<typename Fun>
class ConstraintFunction : public Constraint, public Fun
{
public:
    virtual ~ConstraintFunction() {}

    void addToSolver(QPSolver& sol)
    {
        sol.addConstraint(this);
        static_cast<Fun*>(this)->addToSolver(sol);
    }

    void addToSolver(const std::vector<rbd::MultiBody>& mbs, QPSolver& sol)
    {
        sol.addConstraint(mbs, this);
        static_cast<Fun*>(this)->addToSolver(sol);
    }

    void removeFromSolver(QPSolver& sol)
    {
        sol.removeConstraint(this);
        static_cast<Fun*>(this)->removeFromSolver(sol);
    }
};

The impact on client code is minimal to non-existent. At worse, it is a matter of removing a few ...

[haudren]

Hum good question... In the distant past, that is to say 783e8ba, MotionConstr used to be a ConstraintFunction<Inequality, Bound>, which made sense as we did not have GenInequality. I guess that this feature is not really needed anymore, but the question is: do we see some possible uses for it in the future ?

[jorisv]

I agree with Hervé. I'm still pretty sure we can find some weird constraint/tasks that can compute efficiently multiple type of constraint.

Also what is the advantage to remove this feature ? The only one I see it's to be able to compile with old visual studio version that don't support variadic template. Did you see other advantage ?

2017-01-13 2:23 GMT+01:00 Hervé Audren notifications@github.com:

Hum good question... In the distant past, that is to say 783e8ba https://github.com/jrl-umi3218/Tasks/commit/783e8baef3eaa7c832fc9be088596ba90b1a1c96, MotionConstr used to be a ConstraintFunction<Inequality, Bound>, which made sense as we did not have GenInequality. I guess that this feature is not really needed anymore, but the question is: do we see some possible uses for it in the future ?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/jrl-umi3218/Tasks/issues/20#issuecomment-272335454, or mute the thread https://github.com/notifications/unsubscribe-auth/AAvh0-WC5AUfNcl0Ji79KSNbKlbFVY4dks5rRtIYgaJpZM4Lhyc0 .

[aescande]

It is only a matter of simplification and readability, nothing more. It can also help a bit to reduce compilation times.

I'm not sold on the fact that we will need to address weird corner cases in Tasks. In my understanding, Tasks is dealing with low-level constraints and having composite constraints would be dealt with at the mc_rtc/mc_ros level. However, if we want to think it further, let's consider all the type of mixes (FUN1, FUN2) we could have:

• FUN1 == FUN2 makes no sense/ is trivially reduced to FUN1
• Ineqality and GenInequality can be treated as GenInequality with no or barely no overhead (depending on the solver: no overhead with LSSOL).
• Equality and one of the two flavors of inequality: this should be treated as a single GenInequality, and the solver should handle the case where lb_i == ub_i (LSSOL does that automatically, and for QLD the handling can be done at the same place double-sided inequalities are transformed to single sided).
• Bound and another type of constraint: it could once again be dealt with as GenInequality, but here it is sub-optimal because we lose the sparse structure of the bound constraints, which can usually be handled efficiently by a QP solver. However, the only case where I can see this mix happen is through the use of slack variables. I don't think the actual framework is ready for this case, because it involves adding variables and so asks for a more involved variable management. Also the use of slack variables is a choice of resolution scheme, not formulation, and as such should not be considered at the task level, but in the solver (i.e. implementing a different version of QPSolver).