Skip to content

angvec2r: reject a zero axis for a non-zero angle - #267

Open
petercorke wants to merge 1 commit into
rai-opensource:masterfrom
petercorke:fix/angvec2r-zero-axis
Open

petercorke wants to merge 1 commit into
rai-opensource:masterfrom
petercorke:fix/angvec2r-zero-axis

Conversation

@petercorke

Copy link
Copy Markdown
Collaborator

Follow-up to #266, which makes UnitQuaternion.AngVec raise ValueError for a non-zero angle about a zero axis. As noted there, the base functions did not behave consistently with that.

Problem

angvec2r tested the axis norm and returned the identity if it was ~0, whatever the angle:

>>> angvec2r(0.5, [0, 0, 0])   # silently the identity

angvec2tr, SO3.AngleAxis/AngVec and SE3.AngleAxis/AngVec inherit this. The docstring already says that tol is for the "zero-rotation case" and that V must have finite length when the angle is non-zero, but the code did not do that.

Change

Test the angle for zero instead:

  • angle zero (within tol eps, via iszero): return the identity, axis ignored
  • otherwise: normalize the axis with unitvec, which raises ValueError for a zero-length axis

Docstring notes for angvec2r and angvec2tr updated (the old θ ~= 0 wording was MATLAB "not equal" and read ambiguously).

Behaviour change

A non-zero angle about a zero-length axis (including a tiny axis below 20 eps) now raises ValueError rather than returning the identity. Non-zero axes of any length are unaffected.

Downstream

I audited the sibling toolboxes. The only affected caller is a line-of-sight helper in RTB that applies AngleAxis to cross(a, b), which is zero for (anti)parallel vectors. It is already wrong for antiparallel vectors today, and SO3.RotatedVector (in the released 1.1.18) is the correct replacement. Tracked in petercorke/robotics-toolbox-python#843, to land before this is released.

Tests

  • test_angvec2r_nonunit_axis, test_angvec2r_zero_axis (rad and deg), additions to test_angvec2tr, and test_AngleAxis_zero_axis for SO3/SE3
  • 3 of the new tests fail on current master and pass with the change; full suite 413 passed, 3 skipped
  • Not run through Black: the project pins 23.10.0 and my environment has a newer version, so I formatted by hand and left it to pre-commit/CI

🤖 Generated with Claude Code

angvec2r returned the identity whenever the axis norm was ~0, whatever
the angle, so a non-zero rotation about no axis silently gave the
identity (angvec2tr, SO3/SE3.AngleAxis and AngVec inherit this).

Test the angle for zero instead, which is what the docstring already
described for `tol`: a zero angle (within tol eps) returns the identity
and ignores the axis; otherwise the axis is normalized with unitvec,
which raises ValueError for a zero-length axis.

Consistent with UnitQuaternion.AngVec in rai-opensource#266.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@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!

This branch has not been deployed

No deployments
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