dfki-ric / movement_primitives

Dynamical movement primitives (DMPs), probabilistic movement primitives (ProMPs), and spatially coupled bimanual DMPs for imitation learning.
https://dfki-ric.github.io/movement_primitives
Other
145 stars 39 forks source link

Phase and forcing_term are not updated inside dmp_open_loop() #26

Closed ilska closed 10 months ago

ilska commented 1 year ago

First of all thank you for sharing this great repo. It has helped me a lot to understand the theory behind DMPs.

I have been doing some experiments with your code and when changing the run_t to a different value from the original execution_time I have detected that the algorithm does not correctly adapt to the new run_t.

If run_t < execution time, the movement does not adapt and it is like the result is cut. If run_time > execution time, during that extra time the last position is kept.

I think the problem is that the phase and forcing_terms are not updated inside dmp_open_loop. It is not done either in dmp_step_euler.

I have tried to fix it myself with no luck, because I completely loose the shape of the resulting curve.

Could you help me with this?

AlexanderFabisch commented 1 year ago

Hi @ilska ,

if you have some example code to reproduce the problem, that would help a lot.

ilska commented 1 year ago

cartesian_data.zip cartesian_dmp_test.zip Result_fig

Here it goes :) I have seen the same behaviour in the orientatios, but to keep it simpler I have started trying to understand the problem only in position.

I really appreciate your help

AlexanderFabisch commented 1 year ago

Ah, sorry. This is actually intended behavior. The purpose of the parameter run_t is not to scale the DMP temporally. DMPs are PD controllers that roughly converge to the goal at the end of the execution time. With run_t you can extend the time that the DMP is executed with the same execution time parameter.

If you want to scale the DMP, you can just change the value of DMP.execution_time. This is not an officially exposed public interface though. In order to make it more explicit. We could rename it to DMP.execution_time_ and document it in the Attributes section of the DMP docstring. Let me know if that works for you.

ilska commented 1 year ago

Yes, that was my objective, to time-scale the DMP. I have tried doing what you have said by doing:

dmp.configure(start_y=Y[0], goal_y=Y[-1]) dmp.execution_time = execution_time*0.5 _, Y_new_faster = dmp.open_loop(step_function='euler') Bun in this case, the new curves totally loose the desired shape:

Resitl_fig_2

AlexanderFabisch commented 1 year ago

OK, that was my mistake. The forcing term has to be updated, of course. Here is my next attempt: https://github.com/dfki-ric/movement_primitives/pull/27

Could you try this? You can use the property:

dmp.execution_time_ = 0.5 * execution_time
AlexanderFabisch commented 1 year ago

Seems to work for me.