| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
…/rodrigues trexp/trlog don't have one dominant wasteful generic call like the isR/trnorm/tr2adjoint/qqmul/qvmul fixes did (rai-opensource#213/rai-opensource#214/rai-opensource#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-rai-opensource#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>
|
⚠️ Please install the Codecov Report❌ Patch coverage is 94.73684% with 1 line in your changes missing coverage. Please review.
📢 Thoughts on this report? Let us know! |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Summary
trexp/trlog are much slower than rodrigues (~20us and ~17us vs ~7us), but unlike the previous three perf PRs (#213, #214, #215) they don't have one dominant wasteful generic-NumPy call to fix - 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, each independently re-validating/re-extracting. This PR pulls out the two safe, verified wins found in that chain; it does not close the full gap to rodrigues - see "further speedup opportunities" below.
Net effect, measured old-vs-new in the same process, min of 9 repeats each (single-run timeit numbers were misleading here - see commit message): isskewa ~1.9x, rodrigues ~1.13x, trexp ~1.05x, trlog ~1.10x. Real, but modest compared to #213-#215.
Further speedup opportunities (not attempted here)
A bigger win on trexp/trlog would mean collapsing the redundant getvector/re-validation layers in the hot path into a more direct, fused implementation - e.g. trexp(se3_matrix) currently re-extracts and re-validates the same data through isskewa -> vexa -> iszerovec -> unittwist_norm independently, each with its own getvector call. That's a larger, riskier change: this code has clearly-deliberate handling for numerically delicate edge cases (near-zero rotation, near-pi singularity, caller-supplied theta), so a fused rewrite needs much more careful edge-case test coverage than the drop-in fixes here. Not scoped or attempted in this PR.
Two smaller, unrelated candidates surfaced but out of scope for this PR:
Test plan
🤖 Generated with Claude Code