Skip to content

perf: compute Twist3.Ad() directly instead of via a throwaway SE3 - #211

Open
petercorke wants to merge 1 commit into
rai-opensource:masterfrom
petercorke:perf/twist3-ad
Open

perf: compute Twist3.Ad() directly instead of via a throwaway SE3#211
petercorke wants to merge 1 commit into
rai-opensource:masterfrom
petercorke:perf/twist3-ad

Conversation

@petercorke

Copy link
Copy Markdown
Collaborator

Summary

  • Twist3.Ad() was return self.SE3().Ad() -- builds a full, validated SE3 object purely to immediately extract its array inside .Ad() and discard the object.
  • Computes the same result directly: smb.tr2adjoint(smb.trexp(self.S, check=False)). Same underlying math (trexp then tr2adjoint), just without the intermediate pose-object construction.
  • No behavior change: bit-identical output for the general case, only the computation path is different.
  • No existing test covered Ad() at all. Added one first, with hand-verified ground truth (not derived from Ad() itself) for pure-rotation (block-diagonal [[R,0],[0,R]]) and pure-translation (skew(t) coupling) cases, plus a cross-check against SE3.Ad() over 20 random transforms -- confirmed it passes unchanged against both the old and new implementation.

Test plan

  • New test_Ad passes against the old implementation (self.SE3().Ad()) -- establishes the correctness baseline
  • Passes unchanged after the optimization -- confirms behavior-preserving
  • pytest tests/ -- full suite, 343 passed, no regressions
  • Benchmarked old vs new in the same process: 35.7us vs 25.6us per call, 1.40x faster, results bit-identical (np.testing.assert_almost_equal)

Ad() was self.SE3().Ad() -- constructs a full validated SE3 object
just to immediately extract its array and discard the object. Computes
the same result directly (trexp then tr2adjoint, skipping the SE3
constructor/validation overhead): bit-identical output, ~1.4x faster.

No existing test covered Ad() at all; added one first (hand-verified
pure-rotation and pure-translation cases, plus a cross-check against
SE3.Ad() over 20 random transforms) and confirmed it passes unchanged
against both the old and new implementation.
@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