Repository navigation
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Thanks very much for the report and the fix, @wpyaoxueros. I've confirmed the bug: on master Two small things, neither blocking:
|
|
Thanks for confirming the bug and for both suggestions. I've updated The four near-zero subtests fail on the previous PR head and pass with the update. Remote validation: focused tests 12 passed plus 8 subtests; full suite 414 passed, 11 skipped plus 63 subtests. Black 23.10.0 and whitespace checks pass. You're also right about the description: the base angle-axis functions currently return the identity for a zero axis, including at a nonzero angle. I've corrected the PR text and left those functions unchanged for your separate alignment work. |
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: ```python >>> 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](https://claude.com/claude-code) Co-authored-by: Claude Sonnet 5.5 <[email protected]>
UnitQuaternion.AngVec(0.5, [1, 2, 3])currently produces a quaternion with norm greater than one and a matrix that is not the requested rotation. Normalize the axis before constructing a nonzero-angle quaternion, while returning the identity for angles considered zero bysmb.iszero()after unit conversion.Away from that zero-angle tolerance, a zero axis raises
ValueError. The base functions (angvec2r,angvec2trandSO3.AngleAxis) currently return the identity for a zero axis, even for a nonzero angle; this PR does not change them. The earlier description incorrectly claimed consistency with those base constructors. Their alignment is separate, as discussed in review.Regression coverage includes norm, angle/axis round-trip, matrix agreement, degrees, unit-axis controls, exact and near-zero angles in both units, and positive/negative small angles outside the tolerance. These controls preserve nonzero rotation with a valid axis and reject a zero axis outside the tolerance.
Validation on the designated Linux workstation, Python 3.11:
54fb3c8: four near-zero subtest failures; the four other new control subtests passed.AI assistance: an AI coding agent helped investigate, implement, test, and review this change. No independent human review is claimed.