Fix trlog exceptional condition - #63
elton-choi wants to merge 1 commit into
Conversation
Sorry, something went wrong.
For the rotation part of trlog, the iseye condition does not catch all conditions where the rotation value is near-zero. I actually got divided-by-zero exception in the general case part while doing some simulation. There are many ways to avoid divided-by-zero exception when we calculate skw = ... / math.sin(theta). But, I recommend you to fix iseye routine. Instead of iseye routine, we can simply check if trace(R) = 1 + 2cos(theta=0) = 3
|
thanks for this, it's elegant to use the trace twice. Can you give me the numeric example where iseye() failed. iseye() applies the tolerance threshold to the norm of the matrix, rather than the trace, and the latter is bigger than the former so the tolerances are not equivalent. On the typing branch I've updated iseye and added a unit test for trlog for this case. Perhaps there should be a test on sin(theta) in the "general case" to truly ensure that you're example can't happen again. |
Sorry, something went wrong.
| trace_R = np.trace(R) | ||
| if abs(trace_R - 3) < tol * _eps: | ||
| # matrix is identity | ||
| if twist: |
There was a problem hiding this comment.
nit: write
return np.zeros((3,)) if twist else np.zeros((3, 3)it is much simpler to read and less lines
sorry has nothing to do with the PR :-)
Sorry, something went wrong.
|
Thank you for your interest in SMTB and for the report here! This was fixed, but by a different mechanism than proposed in this PR. #63 targets the smb.iseye(R) identity pre-check, but that whole branch was removed in a9fc08a ("rework code to be more robust to nearly identity rotation matrix") and replaced with a guard placed directly at the point of division (sin(theta) == 0), which is a more robust fix than checking R's distance from identity beforehand. 5d1044a added a complementary fix for an acos domain error in the same neighbourhood. I've opened #208 to add regression coverage for exactly this near-identity case, referencing this PR — confirmed it reproduces the original FloatingPointError against the code as it stood when this was opened, and passes cleanly against current master. Closing as superseded, but appreciate you flagging it — it pointed at a real gap that's now got a test pinned to it. |
Sorry, something went wrong.
For the rotation part of trlog, the iseye condition does not catch all conditions where the rotation value is near-zero. I actually got divided-by-zero exception in the general case part while doing some simulation.
There are many ways to avoid divided-by-zero exception when we calculate skw = ... / math.sin(theta). But, I recommend you to fix iseye routine.
Instead of iseye routine, we can simply check if trace(R) = 1 + 2cos(theta=0) = 3