Repository navigation
Fix trlog exceptional condition - #63
elton-choi wants to merge 1 commit into
Conversation
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 On the Perhaps there should be a test on |
| 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 :-)
|
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 I've opened #208 to add regression coverage for exactly this near-identity case, referencing this PR — confirmed it reproduces the original Closing as superseded, but appreciate you flagging it — it pointed at a real gap that's now got a test pinned to it. |
## Summary - `trlog`'s general-case branch (SO(3)) divides by `sin(theta)`, computed from `trace(R)` via `acos`. The only existing coverage (`test_trlog`) used `R = np.eye(3)` exactly — never a numerically near-identity matrix as produced by real computation. - That gap is exactly what #63 reported: a near-identity `R` reaching the general case and dividing by (effectively) zero. - Confirmed against the pre-`a9fc08a` code (*"rework code to be more robust to nearly identity rotation matrix"*) that `rotx(1e-9)` — not caught by `iseye()`, and not exactly `np.eye(3)` — raises `FloatingPointError: divide by zero encountered in divide` there under strict numpy error settings. Current code's `st == 0` guard (added in `a9fc08a`) plus the `acos` domain clamp (added in `5d1044a`) handle it cleanly. - Also adds a case at `theta=1e-7` (small but not clamped) to confirm the general-case formula stays numerically accurate rather than needing a coarse fuzzy-tolerance fallback. ## Test plan - [x] `pytest tests/base/test_transforms3d.py -k trlog -v` — both tests pass - [x] `pytest tests/base/test_transforms3d.py` — full file, 30 passed - [x] Verified the new case reproduces a real `FloatingPointError` against the pre-`a9fc08a` code path
3 participants
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