Proper multilinear interpolation for both value and gradient #6
Conversation
| typename PD, typename RS, typename RD, typename B> | ||
| VectorXd MatlabValueFunction<TS, TC, TD, PS, PC, PD, RS, RD, B>:: | ||
| RecursiveGradientInterpolator(const VectorXd& x, size_t idx) const { | ||
| return RecursiveMultilinearInterpolator(x idx, true); |
There was a problem hiding this comment.
Ah crap this is a typo, will fix!
| VectorXd DirectionToCenter(const VectorXd& x) const; | ||
|
|
||
| // Accessor for precomputed value at the given state. | ||
| VectorXd ValueAccessor(const VectorXd& x) const; |
There was a problem hiding this comment.
Shouldn't this be outputting a double?
| const size_t idx = StateToIndex(x); | ||
|
|
||
| VectorXd value(x.size()); | ||
| for (size_t ii = 0; ii < value.size(); ii++) value(ii) = data_[ii][idx]; |
There was a problem hiding this comment.
This won't work. Remember, data_ is a std::vector<double>, i.e. it's flattened. Also value output should be a double.
| // Recursive helper function for value or gradient multilinear interpolation. | ||
| // Takes in a state and index along which to interpolate, and a boolean | ||
| // determining whether this is for the gradient or, otherwise, for the value. | ||
| VectorXd RecursiveMultilinearInterpolator(const VectorXd& x, size_t idx, |
There was a problem hiding this comment.
Ooooooh cool idea. Instead of passing in a bool flag, could template the function itself on the accessor function, like:
template <typename Accessor, typename OutputType>
OutputType RecursiveMultilinearInterpolator(const VectorXd& x, size_t idx) const;
There was a problem hiding this comment.
In any case, since the output type can be either double or VectorXd you need to template that.
dfridovi
left a comment
dfridovi
left a comment
There was a problem hiding this comment.
Main issue is that Value should output a double, not a VectorXd. Also suggesting a cleaner template approach to the recursive helper.
dfridovi
force-pushed
the
dynamic_planner
branch
from
2ded36b to
e002e7d
Compare
dfridovi
force-pushed
the
jfisac/interpolator
branch
from
1c1a269 to
9d1adbb
Compare
Added some wrappers for RecursiveGradientInterpolator (and now also RecursiveValueInterpolator) to also work.
Be sure to take a look, I tried not to break anything but let's make sure!