[LinuxCNC/linuxcnc Issue#379] posemath library is incorrect

#1 – andypugh 于 2017-12-16

It seems that this issue has been mentioned before:
https://forum.linuxcnc.org/forum/30-cnc-machines/26392-the-study-of-arithmetic

Where would we expect to see problems if this really is wrong?

#2 – lethang12cdt 于 2017-12-16

In my RPY rotation matrix that i mentioned, when pitch(P) = +- pi/2, we
cant compute roll and yall by fomular rpy->r = atan2(m->y.z, m->z.z); rpy->y
= atan2(m->x.y, m->x.x);
They are always atan2(0,0)

2017-12-16 18:08 GMT+07:00 andypugh :

> It seems that this issue has been mentioned before:
> https://forum.linuxcnc.org/forum/30-cnc-machines/26392-
> the-study-of-arithmetic
>
> Where would we expect to see problems if this really is wrong?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/LinuxCNC/linuxcnc/issues/379#issuecomment-352176631>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AP9LHYTUA71Y_-gmsm8Ty9FJyLZYcU26ks5tA6SbgaJpZM4RAtfH>
> .
>

—
Lê Thắng
Phone: (+84) 1222443855
Email: lethang12cdt@gmail.com

#3 – jepler 于 2017-12-16

According to the I changed the body of the function in question, pmMatRpyConvert to just abort(). The result of the testsuite:

> Runtest: 202 tests run, 202 successful, 0 failed + 0 expected

in other words, our normal testing doesn’t even call this function.

It appears that the function is used indirectly by pumakins, though. pumakins has no tests.

#4 – andypugh 于 2017-12-16

I have a feeling that pumakins is also often reported to be flaky.

#5 – rene-dev 于 2017-12-16

is pumakins not just a less generic version of genserkins?

#6 – andypugh 于 2017-12-16

I am not particularly adept at tracing includes, but it doesn’t seem to me that genserkins uses posemath at all.

#7 – lethang12cdt 于 2017-12-18

i found out that genserkins uses gomath library, the “gomatrpy_convert”
function is similar “pmMatRpyConvert”

2017-12-17 6:47 GMT+07:00 andypugh :

> I am not particularly adept at tracing includes, but it doesn’t seem to me
> that genserkins uses posemath at all.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/LinuxCNC/linuxcnc/issues/379#issuecomment-352220178>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AP9LHV1IP-Qs5FovtAFC6_LkXwssPt2Jks5tBFajgaJpZM4RAtfH>
> .
>

—
Lê Thắng
Phone: (+84) 1222443855
Email: lethang12cdt@gmail.com

#8 – andypugh 于 2017-12-18

gomath uses GOPI2 in the same conditional, but that is defined as PI/2
https://github.com/LinuxCNC/linuxcnc/blob/af15a4d90e1d51d5309db65fe1c9511e486df411/src/libnml/posemath/gomath.h#L52

So at least one of them is probably wrong.
(and has been for about 10 years)

#9 – rene-dev 于 2017-12-18

maybe the 2 can be merged together? or replaced by a dependency? http://glm.g-truc.net/0.9.8/api/modules.html

#10 – jepler 于 2017-12-18

No, we cannot depend on a C++ library such as glm here, because this code needs to be compiled to run in Linux kernel space (for some users and configurations), and without unmaintainable trickery we can’t do that with C++ code.

Yes, the duplication in posemath and gomath is unfortunate, all the more so because both originated at NIST in different times; The ancestor of LinuxCNC itself originated at NIST as EMC and always used posemath. gomath was added later when genserkins was added. Neither posemath nor gomath really seem to be maintained. I checked the last NIST posemath release and it has the exact same code as LinuxCNC does today for pmMatRpyConvert. https://github.com/usnistgov/rcslib/blob/master/src/posemath/_posemath.c seems to be an official mirror based on the github username.

#11 – rene-dev 于 2017-12-18

why does it need to run in kernel space? everything can run in userspace with rt preempt.

#12 – jepler 于 2017-12-18

Because we support RTAI in a mode which uses kernel modules. Yes, if someday we drop this mode, then we can revisit the decision about C++ in realtime components.

#13 – rene-dev 于 2017-12-18

maybe genserkins needs a test, would be interesting if it behaves differently with the change.

#14 – rene-dev 于 2017-12-18

is there anything that works with RTAI, but not rt preempt? In my experience rtai is troublesome with newer kernels and 64 bit.

#15 – jepler 于 2017-12-18

[this is not the place for a referendum on RTAI]

#16 – lethang12cdt 于 2017-12-18

The function “pmMatZyxConvert” have same error like “pmMatRpyConvert”.
https://github.com/LinuxCNC/linuxcnc/blob/master/src/libnml/posemath/_posemath.c#L522

2017-12-18 20:11 GMT+07:00 Jeff Epler :

> [this is not the place for a referendum on RTAI]
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/LinuxCNC/linuxcnc/issues/379#issuecomment-352421480>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AP9LHc8mfNyF9hftF19Up5V1lPxNwK5aks5tBmSagaJpZM4RAtfH>
> .
>

—
Lê Thắng
Phone: (+84) 1222443855
Email: lethang12cdt@gmail.com

#17 – pkmcnc 于 2017-12-18

@lethang12cdt
Using pmMatRpyConvert after pmRpyMatConvert should produce the original vector. If not, make the proposed change and test again.

#18 – jepler 于 2017-12-20

actually it appears I’m mistaken, upstream has the same fix that this bug is proposing. I failed at reading the source. (just taking a copy of upstream fails, as they’ve incompatibly changed their APIs in the last hundred years)

I still wish we had a test.

#19 – jepler 于 2017-12-20

(Despite the affects-2.7 label, I have fixed this in master only)

#20 – lethang12cdt 于 2017-12-25

the gomath.c library has errors too:
https://github.com/LinuxCNC/linuxcnc/blob/master/src/libnml/posemath/gomath.c#L423
https://github.com/LinuxCNC/linuxcnc/blob/master/src/libnml/posemath/gomath.c#L443
replace “GOPI2″ to “-GOPI2″ for conditions.

there is an error in genserkin.c too, in main() function add value of
interation for KinematicInverse function, if not it’s wont work when we
test in user mode
recommend: add
KINSPTR->maxiterations = GENSERDEFAULTMAX_ITERATIONS;

at:
https://github.com/LinuxCNC/linuxcnc/blob/master/src/emc/kinematics/genserkins.c#L724

2017-12-20 11:01 GMT+07:00 Jeff Epler :

> (Despite the affects-2.7 label, I have fixed this in master only)
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/LinuxCNC/linuxcnc/issues/379#issuecomment-352958885>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AP9LHameHhfEzOU6E6YqIq_J3N8Ik7y5ks5tCIatgaJpZM4RAtfH>
> .
>

—
Lê Thắng
Phone: (+84) 1222443855
Email: lethang12cdt@gmail.com

#21 – jepler 于 2017-12-25

Please open pull requests for the problems you identified. Please open one pull request for each separate problem. I would call the things you identified on the previous message two separate problems.