search

andreadelprete / consim

GNU General Public License v3.0
14 stars 1 forks source link

Bug in exponential integrator without double integral #14

Closed andreadelprete closed 4 years ago

andreadelprete commented 4 years ago

Hi @hammoudbilal

I think I've found a bug in the exponential integrator, in the branch of the code dealing with the case in which contact forces are slipping. In this case, you don't compute the double integral of the forces, but you simply saturate the average contact forces computed through the first integral, and then assume that those forces have been constant through the time step. Here it is in the code of your pull request.

I think the mistake is this line:

vMean_ = v_ + sub_dt* dvMean_;

which should be:

vMean_ = v_ + 0.5*sub_dt* dvMean_;

This is also coherent with the equations I had written in the paper (eq. 24 in particular).

I've actually discovered this bug by mistake, simply because I was running some tests to see how not using the second integral could affect accuracy, and here it is what I've found: exp_int_without_double_integral_after_bug_fix According to this result, it seems that the double integral is basically useless for small time steps (<=0.3ms) but rather important for larger time steps. In conclusion, I think we should try to use the double integral also when contacts are slipping! for this reason, I've just modified the theory in the paper to do so (at least in the first method to handle friction cones). It's actually pretty simple. Let me know if you have questions though.

hammoudbilal commented 4 years ago

Hi @andreadelprete

thanks for pointing this out, just by looking at it I found another bug in the case of no contact here, dv_ should be replaced with dvMean_, I will fix both and check again for consistency

I am currently adding a static walk, a trot and a jump examples, this will help me pinpoint any other bugs that might remain.

andreadelprete commented 4 years ago

Which version of the code are you using? The branch bhammoud? Should we merge your PR so that we work all on master? I don't remember what the status of the PR is actually...

hammoudbilal commented 4 years ago

there were a couple of comments on the previous pr that I had to address, so I created a different branch out of it to add the slipping optimization and contact force prediction, bhammoud_qp, I will push the new solo12 gaits simulations, then maybe I can create a new PR and we could completely ignore the previous one ?

andreadelprete commented 4 years ago

ok for me

hammoudbilal commented 4 years ago

this was solved can we close it ?