Search code examples
c++eigeneigen3

Implement Clip() in Eigen


I have code that clips some value to be between a range centered around 0 like below.

Eigen::VectorXd a;
Eigen::VecotrXd b;
a = a.cwiseMin(b).cwiseMax(-b);  // no temporary created here?

I want to factor out the logic into a function. One solution:

Eigen::VectorXd Clip(const Eigen::VectorXd& a, const Eigen::VectorXd& b);

a = Clip(a, b);

But I assume this is inefficient as it creates an extra temporary?

Another solution:

void Clip(Eigen::Ref<Eigen::VectorXd> a, const Eigen::VectorXd& b) {
  a = a.cwiseMin(b).cwiseMax(-b);
}

But this seems inconvenient to use sometimes:

void SomeFunctionSignatureICannotChange(const Eigen::VectorXd& a, const Eigen::VectorXd& b) {
  // Eigen::VectorXd a_clipped = Clip(a, b); would be cleaner.
  Eigen::VectorXd a_clipped;
  Clip(a_clipped, b);
}

The best solution I can think of:

template <typename DerivedV, typename DerivedB>
auto Clip(const Eigen::ArrayBase<DerivedV>& v,
          const Eigen::ArrayBase<DerivedB>& bound)
          -> decltype(v.min(bound).max(-bound)) {
  return v.min(bound).max(-bound);
}

(I assume 'auto' in this case is fine and not the one that common pitfalls warned against?)

However, the code seems template-heavy and a bit-complicated. For example, trailing return type is discouraged by google style guide here:

Use the new trailing-return-type form only in cases where it's required (such as lambdas) or where, by putting the type after the function's parameter list, it allows you to write the type in a much more readable way. The latter case should be rare; it's mostly an issue in fairly complicated template code, which is discouraged in most cases.

Alternatively, if I remove the trailing return type, function return type deduction will kick in. But google style guide seems to also discourage function return type deduction in public headers here

Furthermore, use it only if the function or lambda has a very narrow scope, because functions with deduced return types don't define abstraction boundaries: the implementation is the interface. In particular, public functions in header files should almost never have deduced return types.

I'm new to Eigen and C++ so not sure if I missed anything. Hope to learn from everyone's comments and suggestions. Thanks!


Solution

  • I confirm that a = a.cwiseMin(b).cwiseMax(-b); does not create any temporary. In your case, using auto return type is highly recommended, and I would also argue that your use case is a perfectly legit exception to the aforementioned rules:

    • Writing the return type explicitly would be a nightmare and error prone.
    • This is a very short function that is expected to be inlined. It is thus expected to be declared and defined at the same time. Therefore the second rule does not not really apply.

    In c++14 you can even omit the redundancy:

    template <typename DerivedV, typename DerivedB>
    auto Clip(const Eigen::ArrayBase<DerivedV>& v,
              const Eigen::ArrayBase<DerivedB>& bound)
    {
      return v.min(bound).max(-bound);
    }