Value_of

Developers
1

This value_of signature here: https://github.com/stan-dev/math/blob/develop/stan/math/prim/mat/fun/value_of.hpp#L45

template <int R, int C>
inline typename Eigen::Matrix<double, R, C> value_of(
  const Eigen::Matrix<double, R, C>& x) {
  return x;
}

The comments say it should compile to a no-op and pass things through. But I don’t think that’s true? It throws away the const qualifier so unless the compiler does some backbends to prove to itself it doesn’t need to, it’s going to go ahead and construct another Eigen::Matrix<double, R, C>.

Anyway I think this should return const reference so that we’re not depending on compiler optimizations to make this avoid unnecessary copies.

2 Likes
2

That’s exactly my experience! This is why my templated GLM code was not running as fast as expected. I only figured out this behaviour after a lot of benchmarking.

I’m really not much of a C++ wizard. Are you suggesting that a fix would be to add a const to the return type?

3

Niiiiice, glad to have a verify on it. I’ll make an issue and a pull.

I was just trying to make sure the stuff in adj_jac_vari could be as efficient as hand-written varis. I just ran the unit tests and they passed with the signature:

template <int R, int C>
inline const Eigen::Matrix<double, R, C>& value_of(
  const Eigen::Matrix<double, R, C>& x) {
  return x;
}

The difference being with the const refs in there,

Eigen::Matrix<double, Eigen::Dynamic, 1> v = value_of(x);

still constructs a new Eigen::Matrix that can be written (cause that is what’s asked of it)

But code like:

auto v = value_of(x);

won’t require a copy and

f(value_of(x));

won’t require a copy if f has a const ref signature.

I assume compiler optimizations could make it fast in certain situations either way but it’s nice to not depend on that stuff I guess.

4

Good find. I am curious what speedup we are going to get from this. This little change could make a decent speedup (if we pour into the code base sufficiently many const declarations wherever we can/should have done it).

5

Oh that would really be great. It would make the GLMs run a lot faster I imagine without having to write more long-winded code. Let me know if you’d like me to review something.

6

It’s possible there’s some change. I think in a lot of cases this has been manually coded around.

Like, for https://github.com/stan-dev/math/blob/develop/stan/math/rev/mat/fun/mdivide_left_spd.hpp

There’s signatures for (var and double being placeholder for matrices of var and double):

f(var, var)
f(double, var)
f(var, double)

Because of this there’s tons of code duplication etc. It’d be nice to avoid this.

edit: And I’m hoping I can make the adj_jac_vari fast enough so even in the most critical cases we can use it instead of writing manual varis. value_of being efficient is important for this.

7

+1 on this causing code duplication.

Should we just make the change to the signature of value_of to return a const ref? Or is there a reason to wait on this/be careful? Are you taking care of it or shall I do it?

8

It’s under review now: https://github.com/stan-dev/math/pull/969 . I’m figuring out how to write tests for it now so it doesn’t get swapped back.