-
Notifications
You must be signed in to change notification settings - Fork 481
Adding a spline joint to the joint collection #2784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: devel
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👋 Hi,
This is a reminder message to assign an extra build label to this Pull Request if needed.
By default, this PR will be build with minimal build options (URDF support and Python bindings)
The possible extra labels are:
- build_collision (build Pinocchio with coal support)
- build_casadi (build Pinocchio with CasADi support)
- build_autodiff (build Pinocchio with CppAD support)
- build_codegen (build Pinocchio with CppADCodeGen support)
- build_extra (build Pinocchio with extra algorithms)
- build_mpfr (build Pinocchio with Boost.Multiprecision support)
- build_sdf (build Pinocchio with SDF parser)
- build_accelerate (build Pinocchio with APPLE Accelerate framework support)
- build_all (build Pinocchio with ALL the options stated above)
Thanks.
The Pinocchio development team.
f464bc1 to
660af02
Compare
cfe3213 to
3a119f8
Compare
3c00669 to
1e5d3d3
Compare
aab0161 to
bf16106
Compare
b4923b1 to
86b40d7
Compare
| SE3 operator()(const JointSpline &) const | ||
| { | ||
| PINOCCHIO_THROW_PRETTY( | ||
| std::invalid_argument, | ||
| "Graph - Joint Spline cannot have a q_ref. Please use the joint offset directly."); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why ? It's strange but I don't get why it's not possible.
| if (!edge.forward) | ||
| PINOCCHIO_THROW_PRETTY( | ||
| std::invalid_argument, "Graph - JointSpline cannot be reversed."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have to think about that.
Right now I'm not very confident about the reversal of this joint.
I don't think creating a ctrlFrames vector with inverse transformation inside will give us the inverse transformation.
| PINOCCHIO_ALIGNED_STD_VECTOR(SE3) ctrlFrames; | ||
| ctrlFrames.push_back(SE3::Identity()); | ||
| ctrlFrames.push_back(SE3(Matrix3s::Identity(), Vector3s(Scalar(0.), Scalar(0.), Scalar(1.)))); | ||
| JointModel jmodel(ctrlFrames, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should create a degree 3 spline so bsplineBasisDerivative and bsplineBasisDerivative2 are called.
| PINOCCHIO_ALIGNED_STD_VECTOR(SE3) ctrlFrames; | ||
| ctrlFrames.push_back(SE3::Identity()); | ||
| ctrlFrames.push_back(SE3(Matrix3s::Identity(), Vector3s(Scalar(0.), Scalar(0.), Scalar(1.)))); | ||
| JointModel jmodel(ctrlFrames, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should create a degree 3 spline so bsplineBasisDerivative and bsplineBasisDerivative2 are called.
| PINOCCHIO_ALIGNED_STD_VECTOR(SE3) ctrlFrames; | ||
| ctrlFrames.push_back(SE3::Identity()); | ||
| ctrlFrames.push_back(SE3(Matrix3s::Identity(), Vector3s(Scalar(0.), Scalar(0.), Scalar(1.)))); | ||
| JointModel jmodel(ctrlFrames, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should create a degree 3 spline so bsplineBasisDerivative and bsplineBasisDerivative2 are called.
unittest/finite-differences.cpp
Outdated
| PINOCCHIO_ALIGNED_STD_VECTOR(pinocchio::SE3) ctrlFrames; | ||
| ctrlFrames.push_back(pinocchio::SE3::Identity()); | ||
| ctrlFrames.push_back(pinocchio::SE3(Eigen::Matrix3d::Identity(), Eigen::Vector3d(0., 0., 1.))); | ||
| JointModel jmodel(ctrlFrames, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should create a degree 3 spline so bsplineBasisDerivative and bsplineBasisDerivative2 are called.
| ConfigVector lb(ConfigVector::Constant(jmodel.nq(), 0.)); | ||
| ConfigVector ub(ConfigVector::Constant(jmodel.nq(), 1.)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only difference between generic test function and specialized one for JointModelSplineTpl is the configuration bound.
Is there way to specialize RandomConfigurationStep to handle that ?
If there is no way, can generate the bounds from TestADOnJoints visitor to avoid duplicating the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can specialize the RandomConfigurationStep, like it's done for CompositeJoint, indeed. I'll do it, and you let me know, what you think.
unittest/finite-differences.cpp
Outdated
| std::cout << "name: " << jmodel.classname() << std::endl; | ||
| JointDataSpline jdata = jmodel.createData(); | ||
|
|
||
| CV q = LieGroupType().randomConfiguration(CV::Zero(), CV::Ones()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only difference here is randomConfiguration generation. Is there a way to handle it in LieGroup ?
Otherwise, it could be nice to generate it from the visitor to avoid duplicating the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i checked, and it requires a new lieGroup, from my understanding. So i will generate bounds from the visitors
| q1 = LieGroupType().randomConfiguration( | ||
| Eigen::VectorXd::Zero(jmodel.nq()), Eigen::VectorXd::Ones(jmodel.nq())); | ||
| q2 = LieGroupType().randomConfiguration( | ||
| Eigen::VectorXd::Zero(jmodel.nq()), Eigen::VectorXd::Ones(jmodel.nq())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, randomConfiguration is the only difference between generic and spline joint.
If there is no way to specialize randomConfiguration, we should add a mechanism to this test to generate a valid randomConfiguration without duplicating the test.
| PINOCCHIO_ALIGNED_STD_VECTOR(SE3) ctrlFrames; | ||
| ctrlFrames.push_back(SE3::Identity()); | ||
| ctrlFrames.push_back(SE3(Eigen::Matrix3d::Identity(), Eigen::Vector3d(0., 0., 1.))); | ||
| JointModel jmodel(ctrlFrames, 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should create an order 3 splines to test all the joint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way that neutral and randomConfiguration return us a valid spline configuration ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
neutral is working, because it returns 0. However if we want random, we need to redefine a lieGroup...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to manage spline with abritrary bounds ?
2e60ca8 to
096a5ec
Compare
|
|
||
| template<typename JointModel_> | ||
| struct init; | ||
| struct JmodelWithBounds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should use a more generic name (if with want to add more attribute in the future).
Maybe JointModelWithParameters.
Description
In this PR, we introduce the new spline joint to pinocchio. This implementation is based on the paper of Lee et al. available here
It allows to define a number of SE3 frames, that will define the movement and dynamics of the joint. It's very useful for biomechanics, but can be used in other applications.
For now the joint configuration has to be bounded between 0 and 1, otherwise the transformation computation will fail.
To do
Future