Skip to content

Commit 764e7f5

Browse files
petercorkeclaude
andcommitted
perf(base): speed up isskewa and cache identity matrix in trexp/trlog/rodrigues
trexp/trlog don't have one dominant wasteful generic call like the isR/trnorm/tr2adjoint/qqmul/qvmul fixes did (#213/#214/#215) - they're deep call chains (trexp -> isskewa -> vexa -> iszerovec -> unittwist_norm -> rodrigues -> skew -> rt2tr -> ishom -> isR) where cost is spread thin across many small layers. Two targeted, verified wins pulled out of that chain: - isskewa (validity check trexp runs on se(3) input) had the same bug ishom had pre-#213: np.linalg.norm on a small fixed matrix, plus an all(S[-1,:] == 0) array-allocation-and-compare for the bottom row. Fixed the same way, ~1.9x faster standalone. - np.eye(3) was rebuilt from scratch on every call in rodrigues (1x), trexp's V-matrix construction (1x), and trlog (2x), despite only ever being used as a read-only operand in an addition/multiplication that produces a new array. Replaced with a module-level constant _EYE3, used only at call sites where it's provably never mutated or returned directly (an aliasing hazard if it were) - the four zero-motion/zero-rotation early-return `np.eye(N)` calls in rodrigues/trexp are untouched, left as fresh arrays, since they hand the object directly to the caller. isskew (the plain, non-augmented so(n) check) was also tried with the same explicit-arithmetic treatment as isskewa, but did NOT show a reliable win under min-of-repeats benchmarking - the only cost it avoids is np.linalg.norm on an already-cheap `S + S.T`, not enough margin to reliably beat by hand-unrolling. Reverted to the original implementation rather than keep an unproven "fix". Net effect, measured old-vs-new in the same process (min of 9 repeats each, to suppress system jitter after an earlier round of misleading single-run numbers): isskewa ~1.9x, rodrigues ~1.13x, trexp ~1.05x, trlog ~1.10x. Real but modest - see the PR description's "further speedup opportunities" section for what a bigger win here would actually require. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent fff7009 commit 764e7f5

2 files changed

Lines changed: 50 additions & 8 deletions

File tree

spatialmath/base/transforms3d.py

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,11 @@
5252

5353
_eps = np.finfo(np.float64).eps
5454

55+
# read-only constant: safe to use directly as an operand in expressions that
56+
# produce a new array (e.g. `_EYE3 + ...`), never in a context that could
57+
# mutate it in place or return it directly to a caller
58+
_EYE3 = np.eye(3)
59+
5560
# ---------------------------------------------------------------------------------------#
5661

5762

@@ -1355,7 +1360,7 @@ def trlog(
13551360
else:
13561361
# general case
13571362
Ginv = (
1358-
np.eye(3)
1363+
_EYE3
13591364
- S / 2
13601365
+ (1 / theta - 1 / math.tan(theta / 2) / 2) / theta * S @ S
13611366
)
@@ -1374,8 +1379,7 @@ def trlog(
13741379
diagonal = R.diagonal()
13751380
k = diagonal.argmax()
13761381
mx = diagonal[k]
1377-
I = np.eye(3)
1378-
col = R[:, k] + I[:, k]
1382+
col = R[:, k] + _EYE3[:, k]
13791383
w = col / np.sqrt(2 * (1 + mx))
13801384
theta = math.pi
13811385
if twist:
@@ -1514,7 +1518,7 @@ def trexp(S, theta=None, check=True):
15141518

15151519
skw = skew(w)
15161520
V = (
1517-
np.eye(3) * theta
1521+
_EYE3 * theta
15181522
+ (1.0 - math.cos(theta)) * skw
15191523
+ (theta - math.sin(theta)) * skw @ skw
15201524
)
@@ -2774,7 +2778,7 @@ def rodrigues(w: ArrayLike3, theta: Optional[float] = None) -> SO3Array:
27742778

27752779
skw = skew(cast(ArrayLike3, w))
27762780
return (
2777-
np.eye(skw.shape[0])
2781+
_EYE3
27782782
+ math.sin(theta) * skw
27792783
+ (1.0 - math.cos(theta)) * skw @ skw
27802784
)

spatialmath/base/transformsNd.py

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,11 @@ def isskew(S: NDArray, tol: float = 20) -> bool: # -> TypeGuard[sonArray]:
408408
409409
:seealso: isskewa
410410
"""
411+
# NB: unlike isR/isskewa, an explicit-arithmetic fast path here did not
412+
# show a reliable win under careful (min-of-repeats) benchmarking - the
413+
# only overhead being avoided is np.linalg.norm on an already-cheap
414+
# `S + S.T`, not enough to reliably beat the scalar-indexing cost of
415+
# unrolling it by hand. Left as the original implementation.
411416
return bool(np.linalg.norm(S + S.T) < tol * _eps)
412417

413418

@@ -436,9 +441,42 @@ def isskewa(S: NDArray, tol: float = 20) -> bool: # -> TypeGuard[senArray]:
436441
437442
:seealso: isskew
438443
"""
439-
return bool(np.linalg.norm(S[0:-1, 0:-1] + S[0:-1, 0:-1].T) < tol * _eps) and all(
440-
S[-1, :] == 0
441-
)
444+
n = S.shape[0]
445+
if n == 4:
446+
# explicit sum-of-squares + scalar bottom-row check avoids the
447+
# generic-dispatch overhead of np.linalg.norm and the array
448+
# allocation + all() of the bottom-row comparison, same as isR/ishom
449+
r00 = S[0, 0] + S[0, 0]
450+
r01 = S[0, 1] + S[1, 0]
451+
r02 = S[0, 2] + S[2, 0]
452+
r11 = S[1, 1] + S[1, 1]
453+
r12 = S[1, 2] + S[2, 1]
454+
r22 = S[2, 2] + S[2, 2]
455+
resid = r00 * r00 + r11 * r11 + r22 * r22 + 2.0 * (
456+
r01 * r01 + r02 * r02 + r12 * r12
457+
)
458+
return bool(
459+
resid < (tol * _eps) ** 2
460+
and S[3, 0] == 0
461+
and S[3, 1] == 0
462+
and S[3, 2] == 0
463+
and S[3, 3] == 0
464+
)
465+
elif n == 3:
466+
r00 = S[0, 0] + S[0, 0]
467+
r01 = S[0, 1] + S[1, 0]
468+
r11 = S[1, 1] + S[1, 1]
469+
resid = r00 * r00 + r11 * r11 + 2.0 * r01 * r01
470+
return bool(
471+
resid < (tol * _eps) ** 2
472+
and S[2, 0] == 0
473+
and S[2, 1] == 0
474+
and S[2, 2] == 0
475+
)
476+
else:
477+
return bool(
478+
np.linalg.norm(S[0:-1, 0:-1] + S[0:-1, 0:-1].T) < tol * _eps
479+
) and all(S[-1, :] == 0)
442480

443481

444482
def iseye(S: NDArray, tol: float = 20) -> bool:

0 commit comments

Comments
 (0)