pantor / ruckig

Motion Generation for Robots and Machines. Real-time. Jerk-constrained. Time-optimal.
https://ruckig.com
MIT License
640 stars 155 forks source link

Non-deterministic outputs based on scope of Ruckig data structure #49

Closed pbeeson closed 2 years ago

pbeeson commented 3 years ago

I was testing out Ruckig (nice package BTW), and noticed that I get different results if I recreate the Ruckig data structure for each new problem to solve rather than overwriting the input/target positions, velocities, and accelerations. I'm sorry I can't be more help than that right now It's late here, and I wanted to file this before stopping for the day. I'm sure you could reproduce by running some predetermined set of inputs/outputs in a loop with the Ruckig data structure inside/outside of the loop.

For me, Its as simple as if the for loop below is at the bottom of the snipp or at the top, where I get different results on repeated solves.

   Ruckig<6> otg {0.001};
   InputParameter<6> input;
   OutputParameter<6> output;

   // Set input parameters
   input.max_velocity = maxVelocities;
   input.max_acceleration = maxAccelerations;
   input.max_jerk = max_joint_jerk_;

   for (std::size_t ii=0; ii < data.size(); ++ii) {
pantor commented 2 years ago

Hi @pbeeson!

When creating the InputParameter instance, not all its members are initialized. In particular, current_position, target_position, and the max_<...> limits need be set explicitly. I mean otherwise the whole position-controlled trajectory generation wouldn't make sense, but I'm also open to initialize them all to zero. Those uninitialized values might explain the non-deterministic output. Can you check that this is not the case here? Otherwise, Ruckig should definitely be deterministic.

gavanderhoorn commented 2 years ago

@pantor: what about initialising all fields to something like a quiet NaN, and then checking they have been properly initialised by the user when they are about to be used?

Using a sentinel value could help distinguish between uninitialised and "some random large value".

pantor commented 2 years ago

That seems to be a good idea, thanks for your input @gavanderhoorn!

pbeeson commented 2 years ago

I definitely am setting all (known) required values. All current, target, and maxes.

So my thought last night was that there is something else (maybe in the output Trajectory) that isn’t getting reset properly.

On Fri, Jul 16, 2021 at 2:03 AM pantor @.***> wrote:

That seems to be a good idea, thanks for your input @gavanderhoorn https://github.com/gavanderhoorn!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pantor/ruckig/issues/49#issuecomment-881199936, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5N7RU6E3UFKOZ6FBJZAXTTX7DS7ANCNFSM5AOY6FYA .

pantor commented 2 years ago

I just played around with the code and your hints, but couldn't reproduce something along the lines of your issue. It would be great if you could share a complete example!

Moreover, what output exactly is non-deterministic? Only the OutputParameter or also the trajectory, e.g. its duration?

pantor commented 2 years ago

Oh, and just to make sure that it's not as simple as that: If you would apply your code snippet to the example.cpp, this would change the trajectory between the first and second execution because the input parameter is updated while stepping through the trajectory. This is done in those lines:

input.current_position = output.new_position;
input.current_velocity = output.new_velocity;
input.current_acceleration = output.new_acceleration;
pbeeson commented 2 years ago

I tracked this down to an indexing bug in my own code that was generating the data. Weirdly it got past all CI tests, but the tiny randomness in a bad memory lookup was magnified by the Ruckig process. Thanks.