Closed aescande closed 7 years ago
Quick remark for the test:
warning: enumeration value ‘Fixed’ not handled in switch [-Wswitch]
So something is a bit fishy, the tests pass on Windows, but fail on Linux... I wonder where the difference lies.
Yes, and it does not seems to be small numeric differences. I'll investigate that
Might have something to do with this:
warning: conversion to ‘int’ from ‘double’ may alter its value [-Wconversion]
if (abs(x) >= taylor_n_bound)
Ok so if you replace abs
with fabs
the tests still fail but with tiny errors:
Running 9 test cases...
/home/herve/dev/RBDyn/tests/AlgoTest.cpp(467): error in "EulerTest": check { q.begin(), q.end() } == { goalQ.begin(), goalQ.end() } failed.
Mismatch in a position 1: 0.707107 != 0.707107
*** 1 failure detected in test suite "Algo"
And
Running 1 test case...
/home/herve/dev/RBDyn/tests/IntegrationTest.cpp(189): error in "JointIntegrationTest": difference{0.0010542} between mbc0.q[1][i]{0.44383554716773121} and mbc.q[1][i]{0.44336814831097465} exceeds 0.001
*** 1 failure detected in test suite "JointTest"
I'm confirming Hervé remark on the test. Switching to std::abs
(and std::sqrt
although it has no effect) mostly fixes the tests. The error can be fixed by reducing the time step (actually, the test passes even with 1100 instead of 1000).
Edit: just saw you pushed a fix, so... cool :+1:
The PR is fine imho but you totally broke the indentation as RBDyn uses tabs and not spaces (you also left a few trailing spaces which is not as big a deal).
Can you fix it easily with Visual Studio? (otherwise if you give me write access to your fork I'll fix the indentation for you)
Also, poke @jorisv concerning planar joints
BTW this breaks the Tasks Mimic joint test. You will need to increase the stiffness of the task to make them pass.
This PR basically corrects the EulerIntegration scheme. Previously, it was done like this:
or corresponding formulas for more complex joints. (
x
is the joint position,v
its velocity anda
its acceleration). This was corresponding to integrate position with constant velocity then velocity with constant acceleration, so that the accelerationa(k)
at instantk
was only influencing the positionx(k+2)
. Now it is done like this:which corresponds to integrating both position and velocity for a constant acceleration.
There are still things to check/correct: