Skip to content

Commit

Permalink
[core/python] Custom return policy to avoid conflits with external mo…
Browse files Browse the repository at this point in the history 
…dules. (#332)

* [core] Fix bug at Robot initialization when defining default flexible joints in the model.
* [core/python] Fix wrong keyword arguments for 'EngineMultiRobot.reset' method.
* [core/python] Add new Boost.Python return policy that can be used to by-pass converter registry if 'convertToPython' has been specified.
* [python] Add some missing typing return types.

Co-authored-by: Alexis Duburcq <alexis.duburcq@wandercraft.eu>
  • Loading branch information
@duburcqa
duburcqa and Alexis Duburcq authored Apr 26, 2021
1 parent 8cc93c8 commit c2af825
Show file tree
Hide file tree
Showing 9 changed files with 115 additions and 157 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
cmake_minimum_required(VERSION 3.10)

# Set the build version
set(BUILD_VERSION 1.6.12)
set(BUILD_VERSION 1.6.13)

# Extract major, minor and patch version
string(REPLACE "." ";" _VERSION "${BUILD_VERSION}")
Expand Down
10 changes: 5 additions & 5 deletions core/include/jiminy/core/Macros.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ namespace jiminy
struct is_vector<std::vector<T> > : std::true_type {};

template<typename T>
constexpr bool is_vector_v = is_vector<T>::value; // `inline` variables are not supported by gcc<7.3
constexpr bool is_vector_v = is_vector<std::decay_t<T> >::value; // `inline` variables are not supported by gcc<7.3

// ========================== is_map ============================

Expand All @@ -106,7 +106,7 @@ namespace jiminy
}

template<typename T>
struct isMap : public decltype(isMapDetail::test(std::declval<T *>())) {};
struct isMap : public decltype(isMapDetail::test(std::declval<std::add_pointer_t<T> >())) {};

template<typename T, typename Enable = void>
struct is_map : std::false_type {};
Expand All @@ -131,7 +131,7 @@ namespace jiminy
}

template<typename T>
struct isEigenObject : public decltype(isEigenObjectDetail::test(std::declval<T *>())) {};
struct isEigenObject : public decltype(isEigenObjectDetail::test(std::declval<std::add_pointer_t<T> >())) {};

template<typename T, typename Enable = void>
struct is_eigen : public std::false_type {};
Expand Down Expand Up @@ -165,7 +165,7 @@ namespace jiminy
}

template<typename T>
struct isEigenVector : public decltype(isEigenVectorDetail::test(std::declval<T *>())) {};
struct isEigenVector : public decltype(isEigenVectorDetail::test(std::declval<std::add_pointer_t<T> >())) {};

template<typename T, typename Enable = void>
struct is_eigen_vector : std::false_type {};
Expand All @@ -183,7 +183,7 @@ namespace jiminy
\
template<typename T> \
struct isPinocchioJoint ## type : \
public decltype(isPinocchioJoint ## type ## Detail ::test(std::declval<T *>())) {}; \
public decltype(isPinocchioJoint ## type ## Detail ::test(std::declval<std::add_pointer_t<T> >())) {}; \
\
template<typename T, typename Enable = void> \
struct is_pinocchio_joint_ ## name : public std::false_type {}; \
Expand Down
3 changes: 2 additions & 1 deletion core/src/robot/Model.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -973,8 +973,9 @@ namespace jiminy
{
// Check if joint name exists
std::string const & frameName = flexibleJoint.frameName;
if (!pncModel_.existFrame(frameName))
if (!pncModelFlexibleOrig_.existFrame(frameName))
{
PRINT_ERROR("Frame '", frameName, "' does not exists. Impossible to insert flexible joint on it.");
return hresult_t::ERROR_GENERIC;
}

Expand Down
6 changes: 3 additions & 3 deletions python/jiminy_py/src/jiminy_py/robot.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ class BaseJiminyRobot(jiminy.Robot):
name than the URDF file will be detected automatically without
requiring to manually specify its path.
"""
def __init__(self):
def __init__(self) -> None:
super().__init__()
self.extra_info = {}
self.urdf_path_orig = None
Expand All @@ -521,7 +521,7 @@ def initialize(self,
mesh_path: Optional[str] = None,
has_freeflyer: bool = True,
avoid_instable_collisions: bool = True,
verbose: bool = True):
verbose: bool = True) -> None:
"""Initialize the robot.
:param urdf_path: Path of the URDF file of the robot.
Expand Down Expand Up @@ -844,5 +844,5 @@ def __del__(self) -> None:
pass

@property
def name(self):
def name(self) -> str:
return self.pinocchio_model.name
59 changes: 54 additions & 5 deletions python/jiminy_pywrap/include/jiminy/python/Utilities.h
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#ifndef UTILITIES_PYTHON_H
#define UTILITIES_PYTHON_H

#include <functional>

#include "jiminy/core/Types.h"
#include "jiminy/core/Macros.h"
Expand Down Expand Up @@ -297,7 +298,7 @@ namespace python
///////////////////////////////////////////////////////////////////////////////

template<typename T>
std::enable_if_t<!is_vector<T>::value
std::enable_if_t<!is_vector_v<T>
&& !is_eigen<T>::value, bp::object>
convertToPython(T const & data, bool const & /* copy */ = true)
{
Expand Down Expand Up @@ -341,7 +342,7 @@ namespace python
}

template<typename T>
std::enable_if_t<is_vector<T>::value, bp::object>
std::enable_if_t<is_vector_v<T>, bp::object>
convertToPython(T & data, bool const & copy = true)
{
bp::list dataPy;
Expand All @@ -353,7 +354,7 @@ namespace python
}

template<typename T>
std::enable_if_t<is_vector<T>::value, bp::object>
std::enable_if_t<is_vector_v<T>, bp::object>
convertToPython(T const & data, bool const & copy = true)
{
bp::list dataPy;
Expand Down Expand Up @@ -398,6 +399,54 @@ namespace python
return configPyDict;
}

template<typename T, bool copy = true>
struct converterToPython
{
static PyObject * convert(T const & data)
{
return bp::incref(convertToPython<T>(data, copy).ptr());
}

static PyTypeObject const * get_pytype()
{
if (is_vector_v<T>) // constexpr
{
return &PyList_Type;
}
else if (std::is_same<T, configHolder_t>::value
|| std::is_same<T, flexibleJointData_t>::value) // constexpr
{
return &PyDict_Type;
}
std::type_info const * typeId(&typeid(bp::object));
bp::converter::registration const * r = bp::converter::registry::query(*typeId);
return r ? r->to_python_target_type(): 0;
}
};

template<bool copy>
struct result_converter
{
template<typename T>
struct apply
{
struct type
{
typedef typename std::remove_reference<T>::type value_type;

PyObject * operator()(T const & x) const
{
return converterToPython<value_type, copy>::convert(x);
}

PyTypeObject const * get_pytype(void) const
{
return converterToPython<value_type, copy>::get_pytype();
}
};
};
};

// ****************************************************************************
// **************************** PYTHON TO C++ *********************************
// ****************************************************************************
Expand Down Expand Up @@ -439,7 +488,7 @@ namespace python
///////////////////////////////////////////////////////////////////////////////

template<typename T>
std::enable_if_t<!is_vector<T>::value
std::enable_if_t<!is_vector_v<T>
&& !is_map<T>::value
&& !is_eigen<T>::value
&& !(std::is_integral<T>::value && !std::is_same<T, bool_t>::value)
Expand Down Expand Up @@ -554,7 +603,7 @@ namespace python
}

template<typename T>
std::enable_if_t<is_vector<T>::value, T>
std::enable_if_t<is_vector_v<T>, T>
convertFromPython(bp::object const & dataPy)
{
using V = typename T::value_type;
Expand Down
31 changes: 8 additions & 23 deletions python/jiminy_pywrap/src/Constraints.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,10 @@ namespace python
&PyConstraintVisitor::setIsEnable)
.add_property("baumgarte_freq", &AbstractConstraintBase::getBaumgarteFreq,
&AbstractConstraintBase::setBaumgarteFreq)
.add_property("jacobian", &PyConstraintVisitor::getJacobian)
.add_property("drift", &PyConstraintVisitor::getDrift)
.add_property("jacobian", bp::make_function(&AbstractConstraintBase::getJacobian,
bp::return_value_policy<result_converter<false> >()))
.add_property("drift", bp::make_function(&AbstractConstraintBase::getDrift,
bp::return_value_policy<result_converter<false> >()))
;
}

Expand All @@ -96,24 +98,6 @@ namespace python
}
}

static bp::object getJacobian(AbstractConstraintBase const & self)
{
// Do not use automatic converter for efficiency
return convertToPython<matrixN_t const>(self.getJacobian(), false);
}

static bp::object getDrift(AbstractConstraintBase const & self)
{
// Do not use automatic converter for efficiency
return convertToPython<vectorN_t const>(self.getDrift(), false);
}

static bp::object getReferenceConfiguration(JointConstraint & self)
{
// Do not use automatic converter for efficiency
return convertToPython<vectorN_t>(self.getReferenceConfiguration(), false);
}

static void setReferenceConfiguration(JointConstraint & self, vectorN_t const & value)
{
self.setReferenceConfiguration(value);
Expand Down Expand Up @@ -150,7 +134,8 @@ namespace python
bp::return_value_policy<bp::copy_const_reference>()))
.add_property("joint_idx", bp::make_function(&JointConstraint::getJointIdx,
bp::return_value_policy<bp::copy_const_reference>()))
.add_property("reference_configuration", &PyConstraintVisitor::getReferenceConfiguration,
.add_property("reference_configuration", bp::make_function(&JointConstraint::getReferenceConfiguration,
bp::return_value_policy<result_converter<false> >()),
&PyConstraintVisitor::setReferenceConfiguration);

bp::class_<FixedFrameConstraint, bp::bases<AbstractConstraintBase>,
Expand Down Expand Up @@ -180,9 +165,9 @@ namespace python
bp::args("self", "first_frame_name", "second_frame_name", "distance_reference")))
.def_readonly("type", &DistanceConstraint::type_)
.add_property("frames_names", bp::make_function(&DistanceConstraint::getFramesNames,
bp::return_value_policy<bp::copy_const_reference>()))
bp::return_value_policy<result_converter<true> >()))
.add_property("frames_idx", bp::make_function(&DistanceConstraint::getFramesIdx,
bp::return_value_policy<bp::copy_const_reference>()))
bp::return_value_policy<result_converter<true> >()))
.add_property("reference_distance", bp::make_function(&DistanceConstraint::getReferenceDistance,
bp::return_value_policy<bp::copy_const_reference>()));

Expand Down
100 changes: 24 additions & 76 deletions python/jiminy_pywrap/src/Engine.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,28 +117,16 @@ namespace python
.def_readonly("iter_failed", &stepperState_t::iterFailed)
.def_readonly("t", &stepperState_t::t)
.def_readonly("dt", &stepperState_t::dt)
.add_property("q", &PyStepperStateVisitor::getPosition)
.add_property("v", &PyStepperStateVisitor::getVelocity)
.add_property("a", &PyStepperStateVisitor::getAcceleration)
.add_property("q", bp::make_getter(&stepperState_t::qSplit,
bp::return_value_policy<result_converter<false> >()))
.add_property("v", bp::make_getter(&stepperState_t::vSplit,
bp::return_value_policy<result_converter<false> >()))
.add_property("a", bp::make_getter(&stepperState_t::aSplit,
bp::return_value_policy<result_converter<false> >()))
.def("__repr__", &PyStepperStateVisitor::repr)
;
}

static bp::object getPosition(stepperState_t const & self)
{
return convertToPython<std::vector<vectorN_t> >(self.qSplit, false);
}

static bp::object getVelocity(stepperState_t const & self)
{
return convertToPython<std::vector<vectorN_t> >(self.vSplit, false);
}

static bp::object getAcceleration(stepperState_t const & self)
{
return convertToPython<std::vector<vectorN_t> >(self.aSplit, false);
}

static std::string repr(stepperState_t const & self)
{
std::stringstream s;
Expand Down Expand Up @@ -192,68 +180,28 @@ namespace python
void visit(PyClass & cl) const
{
cl
.add_property("q", &PySystemStateVisitor::getPosition)
.add_property("v", &PySystemStateVisitor::getVelocity)
.add_property("a", &PySystemStateVisitor::getAcceleration)
.add_property("command", &PySystemStateVisitor::getCommand)
.add_property("u", &PySystemStateVisitor::getTotalEffort)
.add_property("u_motor", &PySystemStateVisitor::getMotorEffort)
.add_property("u_internal", &PySystemStateVisitor::getInternalEffort)
.add_property("u_custom", &PySystemStateVisitor::getCustomDynamicsEffort)
.add_property("q", bp::make_getter(&systemState_t::q,
bp::return_value_policy<result_converter<false> >()))
.add_property("v", bp::make_getter(&systemState_t::v,
bp::return_value_policy<result_converter<false> >()))
.add_property("a", bp::make_getter(&systemState_t::a,
bp::return_value_policy<result_converter<false> >()))
.add_property("command", bp::make_getter(&systemState_t::command,
bp::return_value_policy<result_converter<false> >()))
.add_property("u", bp::make_getter(&systemState_t::u,
bp::return_value_policy<result_converter<false> >()))
.add_property("u_motor", bp::make_getter(&systemState_t::uMotor,
bp::return_value_policy<result_converter<false> >()))
.add_property("u_internal", bp::make_getter(&systemState_t::uInternal,
bp::return_value_policy<result_converter<false> >()))
.add_property("u_custom", bp::make_getter(&systemState_t::uCustom,
bp::return_value_policy<result_converter<false> >()))
.add_property("f_external", bp::make_getter(&systemState_t::fExternal,
bp::return_internal_reference<>()))
.def("__repr__", &PySystemStateVisitor::repr)
;
}

static bp::object getPosition(systemState_t const & self)
{
// Do not use automatic converter for efficiency
return convertToPython<vectorN_t>(self.q, false);
}

static bp::object getVelocity(systemState_t const & self)
{
// Do not use automatic converter for efficiency
return convertToPython<vectorN_t>(self.v, false);
}

static bp::object getAcceleration(systemState_t const & self)
{
// Do not use automatic converter for efficiency
return convertToPython<vectorN_t>(self.a, false);
}

static bp::object getCommand(systemState_t const & self)
{
// Do not use automatic converter for efficiency
return convertToPython<vectorN_t>(self.command, false);
}

static bp::object getTotalEffort(systemState_t const & self)
{
// Do not use automatic converter for efficiency
return convertToPython<vectorN_t>(self.u, false);
}

static bp::object getMotorEffort(systemState_t const & self)
{
// Do not use automatic converter for efficiency
return convertToPython<vectorN_t>(self.uMotor, false);
}

static bp::object getInternalEffort(systemState_t const & self)
{
// Do not use automatic converter for efficiency
return convertToPython<vectorN_t>(self.uInternal, false);
}

static bp::object getCustomDynamicsEffort(systemState_t const & self)
{
// Do not use automatic converter for efficiency
return convertToPython<vectorN_t>(self.uCustom, false);
}

static std::string repr(systemState_t & self)
{
std::stringstream s;
Expand Down Expand Up @@ -426,7 +374,7 @@ namespace python
void (EngineMultiRobot::*)(bool_t const &, bool_t const &)
>(&EngineMultiRobot::reset),
(bp::arg("self"),
bp::arg("remove_all_forces") = false,
bp::arg("reset_random_generator") = false,
bp::arg("remove_all_forces") = false))
.def("start", &PyEngineMultiRobotVisitor::start,
(bp::arg("self"), "q_init_list", "v_init_list",
Expand Down Expand Up @@ -503,7 +451,7 @@ namespace python
.add_property("systems", bp::make_getter(&EngineMultiRobot::systems_,
bp::return_internal_reference<>()))
.add_property("systems_names", bp::make_function(&EngineMultiRobot::getSystemsNames,
bp::return_value_policy<bp::return_by_value>()))
bp::return_value_policy<result_converter<true> >()))
.add_property("stepper_state", bp::make_function(&EngineMultiRobot::getStepperState,
bp::return_internal_reference<>()))
.add_property("is_simulation_running", &PyEngineMultiRobotVisitor::getIsSimulationRunning)
Expand Down
Loading

0 comments on commit c2af825

Please sign in to comment.