Repository navigation
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
petercorke
left a comment
There was a problem hiding this comment.
Thanks for this. The broadcast semantics match the existing docstrings, the tests pass locally (367 passed), and round trips against expm agree. It also merges cleanly with the integration hub (#246) and with #249.
One pre-existing issue turned up while reviewing, which this PR sidesteps in Twist2.SE2 but does not fix: the isprismatic property raises AttributeError on any twist sequence. I'll follow up with a separate PR on top of this one.
|
Thank you for your interest in SMTB and for this contribution. Welcome, and congratulations on your first PR here! I've approved this. The broadcast semantics match what the |
Twist3.expdocuments applying one integration step to every twist in a sequence, but currently raisesValueError: length mismatch. TheSE3conversion also rejects that case. The correspondingTwist2methods fail too, including matched per-twist steps inSE2.Handle the shared scalar step in all four methods, and match individual steps in
Twist2.SE2. Check degree-mode warnings per twist so that path works for sequences as well. Singleton expansion and mismatched-length rejection are retained.Validation: 20 focused cases compare the results to independent SciPy matrix exponentials (17 fail before). A 717-row camera-reconstruction fixture exercises 96 motion increments in 2D and 3D: all 21 formerly aborting sequence workflows complete, and transformed points agree with the independent reference within
4e-14. Ordinary single-twist output hashes are unchanged. Full suite: 367 passed, 3 existing skips; project pre-commit hooks pass. Linux/Python 3.12.