Skip to content

perf(base): speed up qqmul and qvmul - #215

Open
petercorke wants to merge 1 commit into
rai-opensource:masterfrom
petercorke:perf/speed-up-quaternion-mul
Open

perf(base): speed up qqmul and qvmul#215
petercorke wants to merge 1 commit into
rai-opensource:masterfrom
petercorke:perf/speed-up-quaternion-mul

Conversation

@petercorke

Copy link
Copy Markdown
Collaborator

Summary

  • qqmul (Hamilton product) and qvmul (vector rotation) both used np.dot/np.cross on 3-vectors, whose generic dispatch overhead - built for arbitrary shapes/broadcasting - dominates cost at this size. Same pattern as isR/trnorm/tr2adjoint in perf(base): speed up isR/ishom/ishom2 orthogonality checks #213/perf(base): speed up trnorm and tr2adjoint #214.
  • qqmul: replaced with explicit scalar Hamilton-product arithmetic. Bit-identical to the prior result (verified to ~1 ULP over 2000 random trials).
  • qvmul: replaced the q * pure(v) * conj(q) sandwich (two full Hamilton products through qqmul/qpure/qconj, each with its own getvector re-validation, and each Hamilton product wasting work on a zero scalar part) with the closed-form rotation identity v' = v + 2s(w x v) + 2 w x (w x v) for q = (s, w). This is a genuinely different (though mathematically equivalent) formula, not just a re-expression of the same one - so it is not bit-identical, verified instead to ~1e-14 absolute over 2000 random trials (with v scaled to magnitude ~10), well within floating-point noise.
  • Net effect: qqmul ~10x faster (15.0us -> 1.4us isolated), qvmul ~24x faster (33.4us -> 1.4us isolated). End to end: Q1 * v drops from ~33us to ~4us (~9x). Q1 * Q2 drops from ~19us to ~10us (~2x) - qqmul itself is no longer the bottleneck there; the remainder is UnitQuaternion construction overhead (in particular qunit()'s np.linalg.norm/np.r_ calls), a separate, un-addressed candidate for a possible future fix in the same spirit, not attempted here.

Test plan

  • pytest tests/ - 343 passed
  • qqmul verified against the prior implementation over 2000 random quaternion pairs (max diff ~1 ULP, i.e. bit-identical modulo floating-point operation ordering)
  • qvmul verified against the prior implementation over 2000 random (quaternion, vector) pairs, vector magnitude ~10 (max abs diff ~1.4e-14)

🤖 Generated with Claude Code

Both used np.dot/np.cross on 3-vectors, whose generic dispatch
overhead - built for arbitrary shapes/broadcasting - dominates cost
at this size, same pattern as isR/trnorm/tr2adjoint (rai-opensource#213, rai-opensource#214).

qqmul: replaced with explicit scalar Hamilton-product arithmetic,
bit-identical to the prior result (verified to ~1 ULP over 2000
random trials).

qvmul: replaced the q * pure(v) * conj(q) sandwich (two full Hamilton
products, each wasting work on a zero scalar part, via qqmul/qpure/
qconj with their own getvector re-validation) with the closed-form
rotation identity v' = v + 2s(w x v) + 2 w x (w x v) for q = (s, w).
This is a different, well-known equivalent formula rather than a
re-expression of the same one, so it is not bit-identical - verified
to ~1e-14 absolute over 2000 random trials (v scaled to magnitude
~10), well within floating-point noise.

qqmul ~10x faster (15.0us -> 1.4us isolated), qvmul ~24x faster
(33.4us -> 1.4us isolated). End to end: Q1 * v drops from ~33us to
~4us (~9x), Q1 * Q2 from ~19us to ~10us (~2x - qqmul itself is no
longer the bottleneck there; the remainder is UnitQuaternion
construction overhead, in particular qunit()'s np.linalg.norm/np.r_
calls, which is a separate, un-addressed candidate for a future fix
in the same spirit).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@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