Skip to content

DHRobot tests fail against SG's new scene-graph cycle detection when sharing link objects across robots #581

Open

Description

@petercorke

New finding, 2026-08-09, while getting the test suite green again after unvendoring spatialgeometry (see the fix/unvendor-spatialgeometry PR) and bumping the effective spatialgeometry API to SG's current main.

tests/test_DHRobot.py::TestDHRobot::test_isspherical fails against SG's main:

ValueError: Cannot set RevoluteDH(...)'s scene_parent to RevoluteDH(...) --
RevoluteDH(...) is already a descendant of RevoluteDH(...), this would
create a cycle in the scene graph

The test builds several DHRobots that reuse the same RevoluteDH link instances in different combinations/orderings:

l0 = rp.RevoluteDH()
l1 = rp.RevoluteDH(alpha=-np.pi / 2)
l2 = rp.RevoluteDH(alpha=np.pi / 2)
l3 = rp.RevoluteDH()

r0 = rp.DHRobot([l0, l1, l2, l3])
r1 = rp.DHRobot([l0, l1])
r2 = rp.DHRobot([l1, l2, l3, l0])   # <-- fails here

Each DHRobot.__init__ sets link.scene_parent = link.parent for every link (BaseRobot._sort_links). SG recently added cycle-detection to scene_parent's setter (a real safety feature -- an undetected cycle in the scene graph previously caused update()'s root-finding walk to spin forever). Because l0/l1/l2/l3 are shared objects, r0's construction already wired l1.scene_parent = l0 (from the [l0,l1,l2,l3] chain); when r2 later tries l0.scene_parent = l3 in a different order, SG correctly notices l0 is already an ancestor of l3 from r0's wiring and refuses, since these are the same link objects, not fresh copies.

Open question, not yet investigated: is this DHRobot pattern (constructing multiple robots from overlapping/reordered sets of shared link objects) supposed to be supported? If so, DHRobot/BaseRobot needs to stop treating scene_parent as if each link belongs to exactly one robot's scene graph (possibly relevant to the Robot/Link kinematic-vs-scene-graph-state decoupling work tracked elsewhere). If not -- i.e. link objects were never meant to be shared across multiple DHRobot instances -- the fix is on the test side (construct fresh link instances per robot) and the test itself is what should change, not the library.

Activity

  1. added
    tech-debtKnown technical debt / deferred cleanup, not a live bug
    on Aug 9, 2026
  2. added a commit that references this issue on Aug 9, 2026
    8dfd09a
  3. petercorke commented on Aug 9, 2026

    @petercorke
    OwnerAuthor

    Fixed pragmatically in #583 (fresh link instances per robot in the test) -- but worth being precise about what that fix actually is: a workaround, not confirmation that the underlying test pattern was wrong.

    Per Peter: links are meant to be stateless (desiderata.md), so sharing the same link instance across multiple robots should be fine. It only breaks today because Link/BaseLink currently inherits SceneNode, so a link's scene_parent is single-valued scene-graph state bolted onto what should be a pure kinematic-model object -- SG's new cycle detection is correctly catching a real structural conflict given that inheritance, not catching bad test design.

    This is the same problem #571 already tracks (decoupling Robot/Link kinematic state from scene-graph/rendering state). Once that lands, test_isspherical can go back to sharing link instances directly across differently-ordered robots -- no fix needed there at that point, just revert the workaround.

    Leaving this issue open (not relying on #583's Fixes #581 to auto-close it) since the actual root cause isn't resolved yet, only worked around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    tech-debtKnown technical debt / deferred cleanup, not a live bug

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions