Skip to content

test: add regression coverage for trlog near-identity division - #208

Open
petercorke wants to merge 1 commit into
rai-opensource:masterfrom
petercorke:test/trlog-near-identity
Open

test: add regression coverage for trlog near-identity division#208
petercorke wants to merge 1 commit into
rai-opensource:masterfrom
petercorke:test/trlog-near-identity

Conversation

@petercorke

Copy link
Copy Markdown
Collaborator

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 Fix trlog exceptional condition #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

  • pytest tests/base/test_transforms3d.py -k trlog -v — both tests pass
  • pytest tests/base/test_transforms3d.py — full file, 30 passed
  • Verified the new case reproduces a real FloatingPointError against the pre-a9fc08a code path

trlog's general-case branch divides by sin(theta), computed from
trace(R) via acos. The only existing test used R = np.eye(3) exactly,
never a numerically near-identity matrix as produced by real
computation -- the case rai-opensource#63 (unmerged) reported hitting a divide-by-zero
on. Confirmed against the pre-a9fc08a code (rework code to be more
robust to nearly identity rotation matrix) that this exact input raised
FloatingPointError there; current code's crisp `st == 0` guard handles
it cleanly.
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants