Skip to content

[DO NOT MERGE] Integration: backlog - #246

Draft
taughz wants to merge 52 commits into
integration/high-priorityfrom
integration/backlog
Draft

taughz wants to merge 52 commits into
integration/high-priorityfrom
integration/backlog

Conversation

@taughz

@taughz taughz commented Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator

Do not merge this PR. It is a hub for integration testing the open PR backlog against Core (internal RAI software). The branch stages the PRs listed below, one --no-ff merge commit per PR in merge order, so they can be tested together. Once Core tests pass, the individual PRs are approved and merged to master in this order (ticking them off here), and this hub is closed.

The branch is rebuilt (force-pushed) whenever a contained PR changes or lands on master. Do not use gh stack sync, gh stack rebase, or gh stack merge on this stack: rebasing flattens the per-PR merge commits, and merging would land the hub itself.

Stack: #245 (high priority) -> this PR (backlog). This PR's diff only shows the backlog PRs; it will be retargeted to master once #245's PRs have all merged.

Core tested: no

Contained PRs (merge order)

Roughly oldest first, with related PRs kept adjacent. Simulated merges show no textual conflicts in this order.

Not included yet

Waiting on a rebase onto master (they conflict with #193, #196, #207); they will be appended here once rebased:

Deferred (decision pending):

Proposed for closure:

petercorke and others added 30 commits August 17, 2026 17:28
Bare except: pass means any exception during the importlib.metadata
lookup gets silently swallowed (not just PackageNotFoundError for
"not installed"), and __version__ never gets set at all in that case --
spatialmath.__version__ raises AttributeError instead of giving
something to print. Narrow the except to PackageNotFoundError and
fall back to an explicit "unknown" so the attribute always exists.
trlog's general-case branch divides by sin(theta), computed from
trace(R) via acos. The only existing test used R = np.eye(3) exactly,
never a numerically near-identity matrix as produced by real
computation -- the case #63 (unmerged) reported hitting a divide-by-zero
on. Confirmed against the pre-a9fc08a code (rework code to be more
robust to nearly identity rotation matrix) that this exact input raised
FloatingPointError there; current code's crisp `st == 0` guard handles
it cleanly.
…one file

spatialmath/base/types.py dispatched on sys.version_info across
_types_35.py, _types_39.py and _types_311.py, from when Python 3.5-3.10
support coexisted. pyproject.toml has required Python >=3.10 since #186,
so _types_35.py could never be reached, and _types_39/_types_311 differed
only in where Self is imported from. Collapse to a single file with a
two-line version check; no change to the names or aliases it exports.
np.linalg.det/norm and np.eye carry large dispatch overhead relative
to the actual work for 2x2/3x3 matrices, dominating SE3/SO3
constructor cost when check=True. Replace with explicit cofactor-
expansion determinants and a squared Frobenius residual for the 2x2
and 3x3 cases (falling back to the generic path otherwise), and
replace the bottom-row `all(... == np.array(...))` checks in
ishom/ishom2 with direct scalar comparisons.

~2x faster isR, ~30-40% faster SE3(T, check=True) construction.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Both functions built small, fixed-size (3x3/4x4/6x6) results out of
generic NumPy helpers (np.cross/unitvec/np.stack in trnorm, np.block
in tr2adjoint) whose dispatch overhead - built for arbitrary shapes
and broadcasting - dominates cost at this size. Same pattern as the
isR/ishom speedup in #213. Replaced with explicit scalar arithmetic
(trnorm) and direct pre-allocated slice-assignment (tr2adjoint),
dtype preserved via np.zeros(..., dtype=T.dtype) so tr2adjoint's
documented SymPy support is unaffected.

~12x faster trnorm, ~2.7x faster tr2adjoint standalone. End to end:
SE3 @ SE3 (which normalizes via trnorm) drops from ~30us to ~4us.
Verified bit-for-bit numeric equivalence against the prior
implementation over 200 random SO(3)/SE(3) trials, plus symbolic
(SymPy dtype=object) equivalence for tr2adjoint.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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 (#213, #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>
…/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>
sphinx-pyrunblock is Peter's own package (published to PyPI, hard fork
of the original WhyNotHugo/sphinx-autorun), reachable at the same
content today via git+https://github.com/petercorke/sphinx-autorun.git
(an old, pre-rename URL that still redirects). Installing the PyPI
wheel directly is ~2-4x faster in CI (no git clone/VCS resolution) and
makes pyproject.toml's declared dependency honest -- previously
"sphinx-autorun" there would, in isolation, resolve to the unrelated
upstream PyPI package of that name rather than Peter's fork.

No functional change: both paths resolve to the same sphinx-pyrunblock
1.1.0 release. Verified with a real sphinx-build against a clean venv.
sphinx-codeautolink's own parsing pass over code blocks is stricter
than plain docutils about this pre-existing 3-space/4-space mismatch
between the :linenos: option and the code content, surfacing a new
'unexpected indent' warning that was previously silently tolerated.
…he root cause they shared

Found via the sphinx-pyrunblock rollout (PR #221): several `.. runblock::`
docstring examples were silently baking broken tracebacks into the
published docs instead of demonstrating real output.

## Twist2.unit() / Twist3.unit()

Branches were swapped (comments didn't match what the code under them
did), `self.w` had a `S.w` typo (undefined name), and the "prismatic"
branch passed a wrong-shape zero argument for Twist2 (3-element list
where a scalar is required). Fixed the branch logic, and use `abs()`
instead of `smb.norm()` for Twist2's scalar `w`.

## Twist2.pole

The docstring's own example called `S.pole()`, but `pole` is a
`@property`, not a method -- `TypeError: 'numpy.ndarray' object is not
callable`. (Twist3.pole's own example was already correct; only its
prose incorrectly said `X.pole()` too.)

## DualQuaternion: a shared root cause across three methods

Fixing `DualQuaternion.SE3()`'s docstring (called `d.T` instead of the
actual `d.SE3()` method) surfaced a real bug: `UnitQuaternion.conj()`
silently re-canonicalized its result's sign (forces scalar part >= 0),
which is correct for constructing a `UnitQuaternion` from arbitrary data
(q and -q are the same rotation) but wrong for conjugation, an algebraic
operation that must satisfy q*conj(q) == 1 for downstream algebra to be
correct.

Fixed `Quaternion.conj()` itself (spatialmath/quaternion.py) rather than
routing around it at each call site -- this is what "conjugate" should
mean regardless of caller. No existing test asserted the old
(re-canonicalizing) behaviour, only the return type.

This one fix resolved three separate, previously-untested DualQuaternion
bugs at once:
- `SE3()`: silently returned a transform with negated translation for
  any rotation with negative quaternion scalar part.
- `norm()`: crashed outright (`math domain error`) for the same reason
  -- also added a small floating-point clamp, since the norm-squared
  terms are mathematically non-negative but rounding can leave e.g.
  -1e-17 instead of exactly 0.
- Vector transformation (`dq * v`): found while re-verifying `norm()`'s
  fix -- unrelated to the conj() sign bug, this affected *every* case
  regardless of sign. The textbook q*P*conj(q) sandwich product's
  translation terms cancel to exactly zero under this class's own
  dual-part embedding convention (`__init__` builds `dual =
  0.5*Pure(t)*real`, translation on the left) -- silently applying only
  the rotation and dropping translation entirely. Since `SE3()` already
  correctly extracts (R, t) from this same embedding, reused it instead
  of hand-deriving a second, convention-specific sandwich formula.

## Test coverage

All of the above had zero prior test coverage (existing DualQuaternion
tests used SE3.Rx(pi/4) -- no translation, positive quaternion scalar
part -- which can't exercise any of these bugs). Added regression tests
for conj()'s sign behaviour (including a mixed-sign multi-valued case),
both branches of Twist2/Twist3.unit(), Twist2/Twist3.pole, and
DualQuaternion's SE3()/norm()/vector-transform with both a
negative-quaternion-scalar case and a general case.

Full suite: 334 passed (up from 326), 0 regressions. Real sphinx-build
verification (isolated venv, before/after): RUNBLOCK-ERROR count 6 -> 0,
warning count unchanged at 14 (confirmed stable across repeat builds of
both branches, ruling out build-to-build nondeterminism).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…oilerplate

Reworks the ad hoc, partly-commented-out timing script (which had been
manually re-enabled with a couple of tweaks, uncommitted, sitting in
the working tree across several perf PRs this session) into something
that's actually a useful reference:

- Split into separate tables, one per category (SO(3)/SE(3) base
  functions, SE3 class, quaternion base functions, UnitQuaternion
  class, twist/exponential-map base functions, Twist3 class, NumPy
  baseline), printed with a plain header between each. Base-level
  (module functions on raw arrays) and class-level (constructors/
  operators/properties on instances) are now consistently separated
  throughout, rather than mixed together - the old Twist3 section in
  particular had base functions (skew, trlog, trexp, rodrigues, ...)
  interleaved with class-level operations.
- Fixed a corrupted docstring header left over from a bad find/replace
  on upstream (`# -*- coding", t)` / `@author", t)`), unrelated to any
  of this but noticed while rewriting the file.
- Fixed mislabelled/ambiguous rows: `inner()` -> `np.inner(s, s).sum()`,
  bare `cross()` -> `base.cross(a, b)` (paired explicitly against
  `np.cross(a, b)`), and rows comparing decomposed-(R,t) vs full-4x4
  composition that had identical labels for different code paths.
- Added coverage that didn't exist before: SE3 <-> RPY, SE3 <-> Euler,
  SE3 -> UnitQuaternion, UnitQuaternion <-> rotation matrix,
  UnitQuaternion -> SE3, and SE3 @ SE3 (normalized compose) alongside
  SE3 * SE3.
- Collapsed the `timeit.timeit(...); result(...)` two-line-per-test
  boilerplate into a single `timeit(stmt, label, setup)` call (module
  import aliased to `_timeit` to free up the name). Replaced the
  single `timeit.timeit(..., number=N)` run per test with
  `min(timeit.repeat(..., number=N, repeat=REPEATS))` - a single long
  run doesn't distinguish real cost from a GC pause or OS scheduling
  hiccup, min-of-repeats does. N dropped from 100_000 to 10_000 (an
  arbitrary starting point either way) since the repeats now do the
  work of suppressing noise that a bigger N was being used for.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The script is a dev tool, not library API; move it out of the installed
package so it no longer ships in the wheel.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CPU, OS, Python, numpy and spatialmath versions and the timing settings,
so a pasted table is self-describing. Modeled on RTB's rne_speed.py.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Both trplot() and trplot2() ended with a bare **kwargs that was never
read anywhere in their bodies, so a typo'd keyword (e.g. framelabel=
instead of frame=) was silently accepted and ignored instead of raising
-- it drew the frame with no error and no label at all. Removing the
dead sink lets Python's own unexpected-keyword-argument check do its
job.

While rewriting the two places that used to lean on this catch-all:

- trplot's anaglyph branch only forwarded a small fixed subset of
  parameters to each eye's recursive call, silently dropping textcolor,
  labels, originsize, origincolor, axislabel, axissubscript, width and
  projection; it also never returned ax and never actually called
  plt.show(block=...), so anaglyph mode ignored block entirely.
- trplot's "T is an iterable of transforms" branch forwarded every
  parameter except axissubscript, and also never returned ax.
- tranimate/tranimate2 were splatting the same unfiltered kwargs dict
  into Animate()/Animate2()'s constructor, the drawing call, and
  run()'s animation-control call, relying on each one's own dead
  **kwargs sink to drop what it didn't need. Once trplot/trplot2
  validate strictly, run()-only parameters (movie, repeat, interval,
  nframes, wait) needed to be split off before the drawing call.

Also fixes a long-standing typo in test_pose2d.py::test_graphics
(T0=T2, which was never a real parameter -- animate()'s documented one
is start=) that this same silent-swallow bug had been quietly masking.

Adds tests/base/test_animate.py: constructing a FuncAnimation under a
non-interactive backend doesn't actually run its per-frame update()
callback (confirmed directly -- Animate2's frame counter never
advances past its initial value), so existing animate() tests only
proved construction didn't raise. These tests force every frame
through the real callback via FuncAnimation.save() (PillowWriter, no
external ffmpeg dependency) and assert trinterp/trinterp2 actually ran
across a real s: 0->1 sweep.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Normalize line directions and moments for parallel and intersection checks. Construct common perpendiculars from the nearest point and unit direction cross product, with regression coverage across Plucker scalings.

Assisted-by: OpenAI Codex (GPT-6 Sol)
Signed-off-by: 李永祺 <doribelove@gmail.com>
Keep the near-parallel tolerance test focused on direction vectors by constructing both lines through the origin. This avoids an unrelated Plucker orthogonality check failure on NumPy 2.

Assisted-by: OpenAI Codex (GPT-6 Sol)
Signed-off-by: 李永祺 <doribelove@gmail.com>
e2h/h2e had one assertion each (N-vector only), with the matrix path and
h2e scaling only exercised indirectly via the pose tests. Add tests for
vector input types, matrix input, per-column scaling, round trips, bad
types, and pin the current (1,N)-is-a-matrix interpretation.

Docstrings: e2h's :seealso: pointed at itself (now h2e); document the
(1,N) interpretation in both.

No behavior change.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
h2e of a (1,N) array, a length-1 vector or a scalar returned an empty
(0,N) array, which is never useful (a homogeneous point has at least two
elements). Raise ValueError instead.

Docstrings for e2h and h2e now state the contract up front: columns are
points, rows never are, with explicit shapes, the (1,N) consequence and
:raises: entries.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Refactor test_animate_nonidentity_start to improve axis geometry validation and streamline assertions.
Added support for stacked numpy arrays in baseposelist.py.
… error

Added support for SE3 constructor with (N, 4, 4) NumPy array.
Add unit test for SE3 constructor with stacked array input.
RoboX2020 and others added 22 commits October 10, 2026 17:04
Add a test to ensure scipy is not loaded when importing spatialmath.

This branch has not been deployed

No deployments
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.

6 participants