Transform.apply inconsistency

[sbarthelemy]

Hi,

I think there is an inconsistency between the Transform.apply overloads. See the following excerpt

class Transform
{
  public:
    // Transformations
    Motion apply(const Motion & mv) const
    {
      return Motion(m_E * mv.w(),
                    m_E * (mv.v() - m_r.cross(mv.w())));
    }

    Motion applyInv(const Motion & mv) const
    {
      vector3d ET_w = m_E.transpose()*mv.w();
      return Motion(ET_w, m_E.transpose()*mv.v() + m_r.cross(ET_w));
    }
    vector3d apply(const vector3d & v) const
    {
      return vector3d(m_E.transpose()*v + m_r);
    }
};

I would prefer having

    vector3d apply(const vector3d & v) const
    {
      return vector3d(m_E*(v - m_r));
   }

   vector3d applyInv(const vector3d & v) const
    {
      return vector3d(m_E.transpose()*v + m_r);
    }

or just removing the vector3d overload, it does not seem to be used anywhere in the project

[olivier-stasse]

Thanks Sebastien I'll look into it and come back to you as soon as possible.

[olivier-stasse]

Hi Sebastien. Thanks for your feedback. I think all the apply(const vector3d &v) from the transform class are code which should be removed.

First if you remove them the code still works.

Second, if vector3d &v is the linear velocity, and assuming a zero angular velocity, the formulas are wrong. If you look at the Handbook of Robotics (HoR) table 2.1 line 1 and 4, the terms with m_r (p in HoR) is multiply to the angular velocity which is zero. Then all the terms with m_r should disappears.

Third, having vector3d multiplied with the class Transform which is well semantically defined as a Plucker Transform looks odd. That is why it is not used in the code. Using the type Motion and Force are better for type checking during compile time.

If you are ok, I will remove the corresponding lines from metapod. If you wish to have a special linear velocity only operator, we should/could define a specific type.

Best, Olivier.

[sbarthelemy]

Hello Olivier,

I'm ok for removing this overload: I have no need for it right now, and it is an easy enough calculation to do.

As a side note, I think the formulae are wrong to transform vectors, because they are meant to transform points. But since we have no types to distinguish points from vector, I think it is best to avoid the confusion and remove the overload.

[olivier-stasse]

commit a701 fix this issue by removing useless apply(const vector3d method) methods.