loco-3d / crocoddyl

Crocoddyl is an optimal control library for robot control under contact sequence. Its solver is based on various efficient Differential Dynamic Programming (DDP)-like algorithms
BSD 3-Clause "New" or "Revised" License
845 stars 173 forks source link

Boost.Python: Optional arguments in overloaded function break Python-derived models #842

Closed wxmerkt closed 4 years ago

wxmerkt commented 4 years ago

Managed to reliably reproduce the issue I referred to in #841. Here is a MWE:

Note, that each of the ddp.solve(...) options below should work the same. However, the only one that calls the calc method in our Python-derived class (which prints x and u and raises an exception to test) is the one with no arguments. The other calls to ddp.solve(xs, us, etc.) never call back into the Python-derived class.

#!/usr/bin/env python
# coding: utf-8
from __future__ import print_function, division

import numpy as np
import crocoddyl

class DifferentialActionModelTestIssue842(crocoddyl.DifferentialActionModelAbstract):
    def __init__(self):
        crocoddyl.DifferentialActionModelAbstract.__init__(self, crocoddyl.StateVector(4), 12, 2)

    def calc(self, data, x, u=None):
        print("[calc]", "x", x, "u", u)
        raise

    def calcDiff(self, data, x, u=None):
        pass

# Creating the DAM
def create_integrated_action_model():
    testDAM = DifferentialActionModelTestIssue842()
    testND = crocoddyl.DifferentialActionModelNumDiff(testDAM, True)
    testIAM = crocoddyl.IntegratedActionModelEuler(testND, 1e-2)
    return testIAM

# Creating the shooting problem
x0 = np.array([0., 0., 0., 0.])
T = 50

terminal_model = create_integrated_action_model()
running_models = [create_integrated_action_model() for _ in range(T)]

# Initial control sequence depends on the gait status:
us_init = [np.ones(running_models[t].nu) for t in range(T)]
xs_init = [np.array(x).copy() for x in np.linspace([0,0], [1,0], T + 1).tolist()]

problem = crocoddyl.ShootingProblem(x0, running_models, terminal_model)

# Solving it using DDP
ddp = crocoddyl.SolverFDDP(problem)
ddp.setCallbacks([crocoddyl.CallbackVerbose()])

# ddp.setCandidate(xs_init, us_init, False) # this is not working due to #841

ddp.solve(xs_init)                           # never calls DifferentialActionModelTestIssue842.calc
ddp.solve(xs_init, us_init)                  # never calls DifferentialActionModelTestIssue842.calc
ddp.solve(xs_init, us_init, 100)             # never calls DifferentialActionModelTestIssue842.calc
ddp.solve(xs_init, us_init, 100, False)      # never calls DifferentialActionModelTestIssue842.calc
ddp.solve(xs_init, us_init, 100, False, 0.1) # never calls DifferentialActionModelTestIssue842.calc
ddp.solve()                                  # Works as intended, calls DifferentialActionModelTestIssue842.calc and raises the exception

# Further, we can break the correctly working `ddp.solve()` option by uncommenting the setCandidate call (another overloaded method) earlier... then it also won't call back into Python
cmastalli commented 4 years ago

I have tested a bit more the issue. If you use crocoddyl.StdVec_VectorX() for constructing xs_init and us_init, then there is no problem, e.g.

xs = crocoddyl.StdVec_VectorX()
us = crocoddyl.StdVec_VectorX()
for x in xs_init:
  xs.append(x)
for u in us_init:
  us.append(u)

ddp.solve(xs)
ddp.solver(xs, us)

I need to investigate a bit more how to solve this issue. It seems that boost option doesn't recognize List object conversion.

cmastalli commented 4 years ago

More updates.

@wxmerkt you have an error in the code, the dimension of the xs_init should be 4, not 2. This is the reason why it doesn't recognize the list. Maybe there is a way to make a warning message for these cases.

wxmerkt commented 4 years ago

@wxmerkt you have an error in the code, the dimension of the xs_init should be 4, not 2.

Indeed, thanks for spotting it! This resolves the observed issue. Interesting that the silent overwriting hence memory messing up had this effect.