Skip to content

Commit 9ee7e8b

Browse files
authored
fix: Twist3 * SpatialVector raised ValueError instead of dispatching (#210)
## Summary - `Twist3.__mul__` raised a hard `ValueError` for any right operand it didn't directly recognize (`Twist3`, `SE3`, scalar), instead of returning `NotImplemented`. That meant Python never fell back to `right.__rmul__(left)`, so `Twist3 * SpatialVelocity` / `SpatialAcceleration` / `SpatialForce` / `SpatialMomentum` all failed unconditionally -- even though `SpatialVector.__rmul__` explicitly documents supporting `Twist3` alongside `SE3` (`isinstance(left, (SE3, Twist3))`). - Found while independently verifying #207 (`SpatialForce`/`SpatialMomentum` coadjoint fix): that PR's `Twist3` code path (`SpatialForce.__rmul__`'s own override) is mathematically correct but was untestable through normal `*` syntax because of this bug -- it only worked via `SE3 *`. - Fix: return `NotImplemented` for unrecognized right operands instead of raising. Genuinely unsupported types (e.g. `Twist3(...) * "hello"`) still fail, now with Python's standard `TypeError` rather than a custom `ValueError`. - Updates `Twist3.__mul__`'s own docstring table, which didn't mention `SpatialVector` as a supported right operand at all. - Fills in the `tests/test_spatialvector.py::test_products` stub (previously just `pass`, with a comment literally noting "twist x v, twist x a, twist x F" as untested) -- checks `Twist3 * X` agrees with the equivalent `SE3 * X` for all four `SpatialVector` subclasses. ## Test plan - [x] New test fails on unfixed code: `ValueError: twist *, incorrect right operand` - [x] Passes after the fix, for all of `SpatialVelocity`/`SpatialAcceleration`/`SpatialForce`/`SpatialMomentum` - [x] Confirmed genuinely unsupported types still fail, now with `TypeError` (Python's standard) instead of the old custom `ValueError` - [x] Grepped `tests/` for anything asserting the old `ValueError` -- nothing depends on it - [x] `pytest tests/` -- full suite, 342 passed, no regressions
1 parent 824a459 commit 9ee7e8b

2 files changed

Lines changed: 27 additions & 4 deletions

File tree

‎spatialmath/twist.py‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1107,14 +1107,18 @@ def __mul__(
11071107
Twist3 scalar Twist3 element-wise product
11081108
scalar Twist3 Twist3 element-wise product
11091109
Twist3 SE3 Twist3 exponential x SE3
1110+
Twist3 SpatialVector SpatialVector adjoint/coadjoint product
11101111
======== ==================== =================== ========================
11111112
11121113
.. note::
11131114
11141115
#. scalar x Twist is handled by ``__rmul__``
11151116
#. scalar multiplication is commutative but the result is not a group
11161117
operation so the result will be a matrix
1117-
#. Any other input combinations result in a ValueError.
1118+
#. ``Twist3 * SpatialVector`` (velocity, acceleration, force, momentum)
1119+
is handled by falling back to
1120+
:meth:`~spatialmath.spatialvector.SpatialVector.__rmul__`.
1121+
#. Any other input combination raises a ``TypeError``.
11181122
11191123
For pose composition the ``left`` and ``right`` operands may be a sequence
11201124
@@ -1147,7 +1151,11 @@ def __mul__(
11471151
# return Twist(left.S * right)
11481152
return Twist3(left.binop(right, lambda x, y: x * y))
11491153
else:
1150-
raise ValueError("twist *, incorrect right operand")
1154+
# not a type we handle directly -- return NotImplemented so Python
1155+
# falls back to right.__rmul__(left), which SpatialVector (velocity,
1156+
# acceleration, force, momentum) defines. If right has no __rmul__
1157+
# either, Python raises its own TypeError.
1158+
return NotImplemented
11511159

11521160
def __rmul__(
11531161
right, left

‎tests/test_spatialvector.py‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,23 @@ def test_products(self):
225225
# v x v = a *, v x F6 = a
226226
# a x I, I x a
227227
# v x I, I x v
228-
# twist x v, twist x a, twist x F
229-
pass
228+
229+
# twist x v, twist x a, twist x F, twist x momentum
230+
T = SE3(1, 2, 3) * SE3.Rx(0.3)
231+
S = Twist3(T)
232+
233+
v = SpatialVelocity([1, 2, 3, 4, 5, 6])
234+
a = SpatialAcceleration([1, 2, 3, 4, 5, 6])
235+
f = SpatialForce([1, 2, 3, 4, 5, 6])
236+
m = SpatialMomentum([1, 2, 3, 4, 5, 6])
237+
238+
# Twist3 * X must agree with the equivalent SE3 * X: same
239+
# underlying transform, two ways of expressing it.
240+
for x in (v, a, f, m):
241+
result = S * x
242+
expected = T * x
243+
self.assertIsInstance(result, type(x))
244+
nt.assert_almost_equal(result.A, expected.A)
230245

231246
def test_spatial_transform_duality(self):
232247
# A pose transform acts on motion (velocity/acceleration) by the

0 commit comments

Comments
 (0)