Skip to content

Add discrete interpollator and fix tests #58

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

Merged
merged 6 commits into from
Jun 4, 2025

Conversation

EtienneAr
Copy link
Collaborator

Hello,

Small PR to add a step function for interpolator.
This will be useful for "interpolating" contacts, or rather choosing the right contact step in the contact sequence based on the timestep and delay.

I also fixed some warnings in the interpolator tests (some variable were declared multiple times...)

@EtienneAr EtienneAr requested a review from edantec May 27, 2025 07:42
@EtienneAr EtienneAr force-pushed the topic/discrete_interpollator branch from 05a969d to 634cc39 Compare May 27, 2025 07:47
Copy link
Collaborator

@edantec edantec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I advise against using templates for interpolation functions, as the need is unclear and it complicates the code.

Eigen::VectorXd q_interp(model.nq);

double delay = 0.02;
interpolator.interpolateConfiguration(delay, timestep, qs, q_interp);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
interpolator.interpolateConfiguration(delay, timestep, qs, q_interp);
interpolator.interpolateConfiguration<Eigen::VectorXd>(delay, timestep, qs, q_interp);

BOOST_CHECK(qs[2].isApprox(q_interp));

delay = 0.5;
interpolator.interpolateConfiguration(delay, timestep, qs, q_interp);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
interpolator.interpolateConfiguration(delay, timestep, qs, q_interp);
interpolator.interpolateConfiguration<Eigen::VectorXd>(delay, timestep, qs, q_interp);

interpolator.interpolateDiscrete(0.39, 0.1, vs, v_interp);
BOOST_CHECK(vs[3].isApprox(v_interp));

interpolator.interpolateDiscrete(0.4, 0.1, vs, v_interp);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
interpolator.interpolateDiscrete(0.4, 0.1, vs, v_interp);
interpolator.interpolateDiscrete<Eigen::VectorXd>(0.4, 0.1, vs, v_interp);

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to compile, self.interpolateConfiguration(delay, timestep, qs, q_interp); should beself.interpolateConfiguration<Eigen::VectorXd>(delay, timestep, qs, q_interp);, same for interpolateState and interpolateLinear.

A proxy for interpolateDiscrete needs to be added.

@EtienneAr
Copy link
Collaborator Author

Ok, I did not see a clean and easy way to handle eigen::ref and have something templated, that doesn't require specifying by hand everytime the template arguments... So I went back to the simple and easy way : everything is expected to be a VectorXd, ecxept contacts that are std::vector

@@ -43,14 +43,14 @@ namespace simple_mpc
const std::vector<Eigen::VectorXd> & vs,
Eigen::Ref<Eigen::VectorXd> v_interp);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest something like this if you want this templated:

    template<int Rows, int Cols, typename MatType
      = Eigen::Matrix<double, Rows, Cols>  
    >
    void interp(
      const double delay,
      const double timestep,
      const std::vector<MatType>& qs,
      Eigen::Ref<MatType> q_interp
    );

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So apparently the problems remains

<...>/simple-mpc/tests/interpolator.cpp: In member function 'void interpolator::interpolate::test_method()':
<...>/simple-mpc/tests/interpolator.cpp:38:42: error: no matching function for call to 'simple_mpc::Interpolator::interpolateConfiguration(double&, double&, std::vector<Eigen::Matrix<double, -1, 1> >&, Eigen::VectorXd&)'
   38 |     interpolator.interpolateConfiguration(delay, timestep, qs, q_interp);
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<...>/simple-mpc/include/simple-mpc/interpolator.hxx:29:10: note: candidate: 'template<class Type, int Rows, int Cols, class MatType> void simple_mpc::Interpolator::interpolateConfiguration(double, double, const std::vector<MatType>&, Eigen::Ref<MatType>)'
   29 |     void interpolateConfiguration(
      |          ^~~~~~~~~~~~~~~~~~~~~~~~
<...>/simple-mpc/include/simple-mpc/interpolator.hxx:29:10: note:   template argument deduction/substitution failed:
<...>/simple-mpc/tests/interpolator.cpp:38:42: note:   'Eigen::Matrix<double, -1, 1>' is not derived from 'Eigen::Ref<MatType>'
   38 |     interpolator.interpolateConfiguration(delay, timestep, qs, q_interp);
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you show the calling code and the current implementation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  pinocchio::urdf::buildModel(urdf_path, JointModelFreeFlyer(), model);
  double timestep = 0.01;
  Interpolator interpolator = Interpolator(model);

  // Test configuration interpolation method
  {
    std::vector<Eigen::VectorXd> qs;
    for (std::size_t i = 0; i < 4; i++)
    {
      Eigen::VectorXd q0(model.nq);
      Eigen::VectorXd q1(model.nq);
      q0 = pinocchio::neutral(model);
      Eigen::VectorXd dq(model.nv);
      dq.setRandom();
      pinocchio::integrate(model, q0, dq, q1);
      qs.push_back(q1);
    }
    Eigen::VectorXd q_interp(model.nq);

    double delay = 0.02;
    interpolator.interpolateConfiguration(delay, timestep, qs, q_interp);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the interpolateConfiguration function itself?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

    template<typename Type, int Rows, int Cols, typename MatType = Eigen::Matrix<Type, Rows, Cols>>
    void interpolateConfiguration(
      const double delay, const double timestep, const std::vector<MatType> & qs, Eigen::Ref<MatType> q_interp)
    {
      assert(("Configuration is not of the right size", qs[0].size() == model_.nq));

      // Compute the time knot corresponding to the current delay
      size_t step_nb = static_cast<size_t>(delay / timestep);
      double step_progress = (delay - (double)step_nb * timestep) / timestep;

      // Interpolate configuration trajectory
      if (step_nb >= qs.size() - 1)
        q_interp = qs.back();
      else
      {
        q_interp = pinocchio::interpolate(model_, qs[step_nb], qs[step_nb + 1], step_progress);
      }
    }

I tried with and without the type template

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strange. You could remove MatType as an argument and write Eigen::Matrix<...> everywhere in its place. Perhaps this messed up the function template parameter deduction...

Copy link
Member

@ManifoldFR ManifoldFR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not perfect, not terrible

approved (stuff can get optimised later)

@edantec edantec merged commit 9da8e74 into main Jun 4, 2025
11 checks passed
@EtienneAr EtienneAr deleted the topic/discrete_interpollator branch June 5, 2025 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants