ros-navigation / navigation2

ROS 2 Navigation Framework and System
https://nav2.org/
Other
2.3k stars 1.2k forks source link

[MPPI]: Replace xtensor with Eigen Experiment #4237

Open guoxxiong opened 3 months ago

SteveMacenski commented 3 months ago

Can you back up that claim or provide any other detail?

guoxxiong commented 3 months ago

Because my hardware does not support xsimd acceleration, and xtensor does not support gcc-7, while other programs rely on gcc-7, so I want to know whether the performance will be weakened by using Eigen instead of xtensor for the tensor of 2000x50.

SteveMacenski commented 3 months ago

That's not sufficient to prevent us from looking into it as a potential course of action.

guoxxiong commented 3 months ago

For a 2000*60 array, run following code, it outputs: dot_product_xt: 240000 xtensor dot product took 0.00535805 seconds. dot_product_eigen: 240000 Eigen dot product took 0.000759429 seconds.

Code:

include

include

include <xtensor/xarray.hpp>

include "/usr/include/eigen3/Eigen/Dense"

constexpr int rows = 2000; constexpr int cols = 60;

int main() {

xt::xarray<double> A_xt({rows, cols}, 1.0);
xt::xarray<double> B_xt({rows, cols}, 2.0);
Eigen::MatrixXd A_eigen(rows, cols);
Eigen::MatrixXd B_eigen(rows, cols);
A_eigen.setOnes();
B_eigen= B_eigen.setOnes() * 2.0;

double dot_product_xt = 0.0;
double dot_product_eigen = 0.0;

auto start_xtensor = std::chrono::steady_clock::now();
dot_product_xt = xt::sum(A_xt * B_xt)();
std::cout << "dot_product_xt: " << dot_product_xt << std::endl;
auto end_xtensor = std::chrono::steady_clock::now();
std::chrono::duration<double> elapsed_xtensor = end_xtensor - start_xtensor;
std::cout << "xtensor dot product took " << elapsed_xtensor.count() << " seconds." << std::endl;

auto start_eigen = std::chrono::steady_clock::now();
dot_product_eigen = (A_eigen.array() * B_eigen.array()).sum();
std::cout << "dot_product_eigen: " << dot_product_eigen << std::endl;
auto end_eigen = std::chrono::steady_clock::now();
std::chrono::duration<double> elapsed_eigen = end_eigen - start_eigen;
std::cout << "Eigen dot product took " << elapsed_eigen.count() << " seconds." << std::endl;

return 0;

}

SteveMacenski commented 3 months ago

That would seem to evidence that Eigen is faster than xtensor, not slower by a factor of over 7x! That would point to the fact that we should consider using Eigen instead of xtensor - which is why its mentioned in the MPPI ticket as something that we should seriously look to evaluate instead of using xtensor for an acceleration boost.

I don't understand your ticket's claim then that xtensor is slower and shouldn't be analyzed?

guoxxiong commented 3 months ago

The xtensor should be analyzed. I just want to try to replace the xtensor in MPPI with eigen, and I do not know whether the original control frequency can be maintained.

SteveMacenski commented 3 months ago

That is not what your initial ticket title implies, but OK - maybe just a misunderstanding.

Yes, I definitely support some help in trying to migrate MPPI to Eigen to see if its an improvement.

and I do not know whether the original control frequency can be maintained.

No one knows to be able to tell you for certain. However, metrics I see make me think that its a worthwhile direction to explore and that it might actually make it run faster. Are you interested in spending some time working on this yourself?

guoxxiong commented 3 months ago

Thanks for your advice and help, I will try to migrate MPPI to Eigen.

SteveMacenski commented 3 months ago

OK. I renamed the ticket to be in line with our discussion.

I'd recommend outright ignoring the critics to get started and focus on the core Optimizer/Noise Generator/utils. I think that should make it clear from benchmarking whether its faster or not than xtensor without going through the more laborious task of migrating the critics (which some are easy, some are not). Also, don't fret if immediately the performance isn't as good; there may be steps we can take to improve things. There were several cycles of optimizations and testing on xtensor to get the performance we have now and I would expect the same from Eigen. However, the benchmark on Eigen views makes me think we can save some compute time.

Do you have a general sense of the priority of this task or when you expect to make some progress? I'd be happy to answer any questions or see how I can help if you work on this on a Nav2 fork that is public.

guoxxiong commented 2 months ago

Hi~, I have initially migrated MPPI to Eigen in Nav1 and conducted a rough comparison experiment. Specifically, Eigen::Array is used instead of xt::xtensor.
Params: model_dt = 0.1, time_step = 32, batch_size = 1800, all Critics Processor: I5 12400F Control loop execution time in Eigen Array (ROS1): average: 5 [ms], max: 7 [ms], min: 3 [ms] . Control loop execution time in xt::xtensor (ROS1): average: 5 [ms], max: 8 [ms], min: 3 [ms]. The conclusion is that using Eigen Array instead of xtensor will not significantly improve the calculation speed.

SteveMacenski commented 2 months ago

Can you share the source? I’d like to spend a day or two playing around to make sure I find the same behavior and see if I can tweak it at all to improve things.

Did you look at using Eigen’s tensor library instead of Array? The tensor views are supposedly much faster from open source benchmarking I’ve seen

guoxxiong commented 2 months ago

source: https://github.com/guoxxiong/mppi_local_planner Because the tensor used by MPPI is at most two-dimensional, I used Array. Eigen : : Tensor may be faster for high-dimensional tensor calculation, but it is more complicated to operate.

SteveMacenski commented 2 months ago

The tensor views have the ability to process information without direct copies, which array I believe lacks. There may be some efficiency gains using Eigen tensor views that arrays don't have. For example, I'm seeing some for loops in your code that could be handled with Tensors that would likely be more efficient by using simd vectorization or even later GPU support

I think it is worth trying to use Eigen Tensor & views over creating everything as a looping function on arrays. That could give you that edge in run-time speed

guoxxiong commented 2 months ago

Yes, when I increased the array to 2000 * 56, Eigen::Array was much slower than xtensor, xtensor(15 ms) vs Eigen::Array(30 ms).

guoxxiong commented 2 months ago

I wonder if Eigen::Ref works, Eigen::Array allows the use of references (Eigen::Ref) to modify elements in an array without data copying, which is similar to xt::view in xtensor.

SteveMacenski commented 2 months ago

I think its worth testing with Eigen Tensors themselves instead of trying to work around them with Arrays https://eigen.tuxfamily.org/dox/unsupported/eigen_tensors.html

SteveMacenski commented 2 months ago

@guoxxiong any thoughts / have some cycles to look into it?

guoxxiong commented 2 months ago

@SteveMacenski ,I used Eigen::replicate, Eigen::Map, Eigen::TensorMap to improve performance, it can indeed achieve better results in CPU occupancy and speed.

SteveMacenski commented 2 months ago

Great! Can you share some metrics and and potential paths forward? :smile:

This would be a great contribution to Nav2 / the community to speed things up!

Ayush1285 commented 3 weeks ago

@SteveMacenski We can go ahead and discuss about using Eigen for MPPI. As mentioned earlier, I've implemented Eigen Array for the calculations in Optimizer, NoiseGenerator, MotionModel, and Util files. Link of the fork repo: https://github.com/Ayush1285/navigation2/tree/main. Changes are in eigen_mppi branch. These are the benchmark results of Eigen and xtensor. (Note - Without any Critics plugin).

Results averaged over 5 runs: Benchmark xtensor Eigen Array
BM_DiffDrivePointFootprint 7.82 ms 6.17 ms
BM_DiffDrive 7.31 ms 6.30 ms
BM_Omni 8.35 ms 8.30 ms
BM_Ackermann 7.51 ms 6.27 ms

Considering that this is my very first draft of the implementation, we can potentially gain significant improvement in compute time for the controller. Although, I'm not sure if we can reduce the time to half.

Few points on which we can improve:

  1. We don't have any xt::roll() like function in Eigen. So, I've assigned the values to a temporary copy, and then I'm reassigning it back to the original variable. For the size of the 2000 * 50 matrix, it could be a very expensive operation. We can potentially write a custom function to use Eigen::Map or something else to improve on this.
  2. Similarly we don't have any function in Eigen to generate random numbers from a normal distribution. I've used Eigen::NullaryExpr with lambda expression to generate noises. We can improve on this also.

There could be many more optimisations possible. We'll have to find out. Looking for your insights!!

SteveMacenski commented 3 weeks ago

Have you taken a look at Eigen tensors? https://eigen.tuxfamily.org/dox/unsupported/eigen_tensors.html I think that should be a source of improvement as well rather than using Array. I think its definitely worth experimenting with that (maybe keep a second branch with tensor to build out in parallal to compare) since that's a big feature that could have major runtime differences. Particularly when striding through the tensors in the critics.

Note @guoxxiong 's comment https://github.com/ros-navigation/navigation2/issues/4237#issuecomment-2084282322 -- though it looks like his repository is no longer public. @guoxxiong can you share this again so we can work on this from where you were at?

That's annoying that there's no roll... luckily that's one of the more simple ones to implement for a first-order level! It looks like the tensor library has a NormalRandomGenerator! But, googling around shows some different options for Eigen-esk things for Array/Matrix

Ayush1285 commented 3 weeks ago

Have you taken a look at Eigen tensors? https://eigen.tuxfamily.org/dox/unsupported/eigen_tensors.html I think that should be a source of improvement as well rather than using Array.

I don't think Eigen Tensor will improve the performance since we are only dealing with 2 dimensions. It will only increase complexities. Even in the Medium Article that you shared link, they have used Eigen::Matrix for linear algebraic calculations. image

SteveMacenski commented 3 weeks ago

I think its worth a try and important to benchmark to make sure its not an improvement. Striding and AVX I don't know about Matrix vs Eigen w.r.t. the benchmark, they only showed Tensor.

We also broke down the tensor into 2D from ND to optimize the performance for xtensor, so we could possibly readd the full dimensionality X,Y,Theta in a single tensor with Eigen if that helps performance in the Eigen migration. So this would either way be a necessary stepping stone. Part of this is experimenting with different configurations of Eigen and see what's the best!

We run this at 50-100hz with thousands of samples in each iteration, so small improvements are a big runtime change - especially in strides/views/slices that we cannot intuit without trying. That's the area a migration like this would possibly get improvements from - which the benchmark shows should be pretty major. I suspect that because the strides/slices are so much faster in Eigen, we can recollapse the tensors so that they contain the X,Y,Theta information (instead of 3 separate 2D tensors) and will be again another big boost to exploit the AVX optimizations for a single iteration through the tensor to do all 3x the work. But, it is definitely worth testing all 3 ways: Array, Tensor 2D, and Tensor ND

Ayush1285 commented 3 weeks ago

But, it is definitely worth testing all 3 ways: Array, Tensor 2D, and Tensor ND

Agreed, without testing it would be inconclusive. I'll move forward with Eigen::Tensor implementation.

they only showed Tensor.

They have mentioned that they have used Eigen::Matrix for benchmarking, not Eigen::Tensor. Change “Tensor” to “Eigen::Matrix<double,num,num>” and you get an Eigen implementation (not exactly as sequences in Eigen are closed sets — they include the ends). Snapshot of the blogpost:

guoxxiong commented 3 weeks ago

Agreed! In addition, I think CPU occupancy should also be taken into account.

guoxxiong commented 3 weeks ago

Note @guoxxiong 's comment https://github.com/ros-navigation/navigation2/issues/4237#issuecomment-2084282322 -- though it looks like his repository is no longer public. @guoxxiong can you share this again so we can work on this from where you were at? @SteveMacenski , So sorry, due to my own professional reasons, this part of the code can not be open source for the time being.

SteveMacenski commented 2 days ago

@Ayush1285 hey! Any updates or findings! :smile:

Ayush1285 commented 1 day ago

@Ayush1285 hey! Any updates or findings! 😄

Hey @SteveMacenski , I wasn't able to work on it due to some personal reasons. I can resume working on it atleast after 2 weeks. Sorry for the delay :)