fix(dynamics): guard Robot.rne() against standard-DH DHRobot instances

Root-caused why Robot.rne() (the separate ETS/Featherstone implementation)
only ever worked for modified DH: its algorithm structurally requires the
joint to be the last element of its own ETS segment (Featherstone's
spatial-vector convention). This holds for Robot/ERobot/URDFRobot (built
via ETS.split()), PoERobot (_update_ets() appends the joint last too), and
-- checked carefully, since it wasn't obvious -- for DHRobot(mdh=True) too:
DHLink._to_ets()'s MDH revolute branch reorders a nonzero d translation to
precede the joint rotation specifically so the joint ET stays last; valid
because a z-rotation and a z-translation about/along the same axis commute.
It does not hold for DHRobot(mdh=False) (standard DH), where the joint
comes before the link's fixed geometry -- structurally incompatible with
the algorithm, not fixable without changing what Robot.rne() fundamentally
assumes.

Rather than teach Robot.rne() a second, DH-aware recursion, it now asserts
on the incompatible case instead of silently returning a wrong answer:
`assert getattr(self, "mdh", True)`. Checked via the mdh attribute rather
than class identity -- an earlier class-name blocklist attempt, and a
considered-and-rejected class-name allowlist, would each have gotten this
wrong (the allowlist specifically would have rejected PoERobot, a fully
compliant type). Design tradeoffs recorded in tech-debt.md, tied to the
pending robot-class-hierarchy redesign since that may make the whole
question moot.

New diagnostic script examples/rne_dh_convention_check.py: a single-link
revolute robot, checked against the Lagrangian-identity ground truth
(independent of both RNE implementations), demonstrating rne_python()
agrees for both DH conventions while Robot.rne() now cleanly rejects
standard DH rather than silently mis-computing it.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

Author: petercorke

examples/rne_dh_convention_check.py

@@ -0,0 +1,77 @@
+#!/usr/bin/env python
+"""Root-cause diagnostic for the Robot.rne() vs DHRobot.rne_python() divergence
+originally seen in rne_compare.py -- see rne.md (issue 6) for the full writeup.
+
+Uses a single-link revolute robot with a pure axis twist (alpha != 0, a=d=0)
+to isolate the frame-convention question from friction/armature/multi-link
+recursion complexity. Ground truth is obtained *independently* of either RNE
+implementation via the Lagrangian identity: for a static pose (qd=qdd=0), the
+required joint torque equals dV/dq, where V(q) = m g z_com(q) is the
+gravitational potential energy of the link's centre of mass, computed
+directly from link.A(q) and numerically differentiated.
+
+Historical note: this originally showed rne_python() wrong for modified DH
+and Robot.rne() wrong for standard DH -- three real bugs in rne_python()'s
+MDH branch (see rne.md) have since been fixed, so rne_python() now agrees
+with ground truth for both conventions. Robot.rne()'s standard-DH case is
+not a bug to fix: its Featherstone recursion structurally requires the
+joint to be the last element of its own ETS segment, which standard DH
+never satisfies (the joint comes first). Robot.rne() now asserts on that
+case (see Robot.py) instead of silently returning a wrong answer.
+"""
+
+import numpy as np
+
+from roboticstoolbox import DHRobot, RevoluteDH, RevoluteMDH
+from roboticstoolbox.robot.Robot import Robot as RobotBase
+
+
+def gravity_torque_truth(robot, q0, g=9.81, h=1e-6):
+    """Numerically-differentiated ground truth, independent of any RNE code."""
+    link = robot.links[0]
+    p_local = np.array([*link.r, 1.0])
+
+    def com_z(q):
+        A = np.asarray(link.A(q))
+        return (A @ p_local)[2]
+
+    return g * (com_z(q0 + h) - com_z(q0 - h)) / (2 * h)
+
+
+def check(label, robot, q0=0.3, robot_rne_expected_to_work=True):
+    z = np.zeros(1)
+    q = np.array([q0])
+    truth = gravity_torque_truth(robot, q0)
+    tau_py = robot.rne_python(q, z, z)[0]
+    print(f"{label:28s} truth={truth:9.4f}  rne_python={tau_py:9.4f}")
+    print(f"{'':28s} |rne_python - truth| = {abs(tau_py - truth):.4f}")
+
+    if robot_rne_expected_to_work:
+        tau_base = RobotBase.rne(robot, q, z, z)[0]
+        print(f"{'':28s} Robot.rne={tau_base:9.4f}")
+        print(f"{'':28s} |Robot.rne - truth| = {abs(tau_base - truth):.4f}")
+    else:
+        try:
+            RobotBase.rne(robot, q, z, z)
+        except AssertionError:
+            print(f"{'':28s} Robot.rne: correctly rejected (AssertionError)")
+        else:
+            print(f"{'':28s} Robot.rne: ERROR -- expected AssertionError, got a result")
+
+
+print("Single-link revolute robot, alpha=1.2 rad, a=d=0, r=[0.5,0,0], static (qd=qdd=0)")
+print("Ground truth = d/dq[ m g z_com(q) ], independent of both RNE implementations.")
+print()
+
+std = DHRobot([RevoluteDH(a=0, alpha=1.2, d=0, m=1.0, r=[0.5, 0, 0])], gravity=[0, 0, -9.81])
+check("Standard DH (mdh=False)", std, robot_rne_expected_to_work=False)
+
+print()
+mdh = DHRobot([RevoluteMDH(a=0, alpha=1.2, d=0, m=1.0, r=[0.5, 0, 0])], gravity=[0, 0, -9.81])
+check("Modified DH (mdh=True)", mdh, robot_rne_expected_to_work=True)
+
+print()
+print("Conclusion: rne_python is correct for both DH conventions.")
+print("Robot.rne is correct for modified DH, and now cleanly rejects (rather")
+print("than silently mis-computing) standard DH, since its Featherstone")
+print("recursion cannot represent standard DH's joint-first structure.")

src/roboticstoolbox/robot/Robot.py

@@ -1617,8 +1617,49 @@ def rne(
         - This version supports symbolic model parameters
         - Verified against MATLAB code
 
+        .. warning::
+
+            Assumes each link's joint is the *last* element of its own ETS
+            segment (Featherstone's spatial-vector convention -- fixed
+            geometry gets you *to* the joint, the joint is the last thing
+            applied before the next link's frame). This is guaranteed for
+            any ``Robot``/``ERobot`` built normally, either from a raw ETS
+            (``Robot.__init__`` splits it via ``ETS.split()``, whose
+            default "last" method enforces this per segment), from a URDF
+            (each link's ETS is built fixed-transform-then-joint, in that
+            order), or from a ``PoERobot`` (``_update_ets()`` appends the
+            joint ET last too). It does **not** hold for a ``DHRobot``
+            using standard DH conventions (``mdh=False``), where the joint
+            comes *first*, followed by ``d``/``a``/``alpha`` -- calling
+            ``Robot.rne(dh_instance, ...)`` directly (bypassing
+            ``DHRobot``'s own correct ``rne()``/``rne_python()``) gives
+            silently wrong answers. A ``DHRobot`` built with ``mdh=True``
+            *is* joint-last (``DHLink._to_ets()``'s MDH branch reorders a
+            revolute link's ``d`` translation to precede the joint
+            rotation -- valid since a z-rotation and a z-translation
+            commute -- so the joint ET is always last regardless of ``d``)
+            and works correctly through this path. See rne.md /
+            tech-debt.md.
         """
 
+        # Checked via the `mdh` attribute rather than isinstance/class name:
+        # joint-last compliance tracks the DH convention actually in use
+        # (DHLink._to_ets() puts the joint last for mdh=True, not for
+        # mdh=False), not the DHRobot class itself -- a DHRobot(mdh=True)
+        # instance is structurally fine here (verified numerically against
+        # rne_python() with nonzero d/alpha/mass/inertia). Non-DHRobot
+        # types have no `mdh` attribute and default (via getattr) to True,
+        # since their construction (ETS.split(), URDF, PoERobot's
+        # _update_ets()) already guarantees joint-last independently of DH
+        # conventions.
+        assert getattr(self, "mdh", True), (
+            "Robot.rne() assumes each link's joint is the last element of "
+            "its own ETS segment, which does not hold for a DHRobot built "
+            "with mdh=False (standard DH). Call DHRobot's own "
+            "rne()/rne_python() instead of Robot.rne(dh_instance, ...) "
+            "directly."
+        )
+
         n = self.n
         # n = len(self.links)
 

tech-debt.md

@@ -57,6 +57,56 @@ in a major version increment, retaining `Robot` as a deprecated alias.
 **Best done alongside:** the ETS/fknm refactor — the type picture simplifies
 dramatically if hierarchy and representation are both cleaned up together.
 
+### Related: `Robot.rne()`'s misuse guard checks `mdh`, not class identity
+
+`Robot.rne()` (`Robot.py`) assumes Featherstone's joint-last-in-segment
+structure. A cheap runtime guard rejects the incompatible case:
+
+```python
+assert getattr(self, "mdh", True), ...
+```
+
+Two design questions came up while adding this guard 2026-07-21, both worth
+recording since they'll resurface if the class hierarchy redesign above
+happens:
+
+1. **Blocklist vs. allowlist.** First attempt was a class-name blocklist
+   (`assert not any(c.__name__ == "DHRobot" for c in type(self).__mro__)`).
+   An allowlist (`Robot`/`ERobot`/`URDFRobot` by name) was considered and
+   rejected: it would have wrongly rejected `PoERobot`, a fully compliant
+   type (its `_update_ets()` also appends the joint ET last) that was easy
+   to overlook — demonstrating exactly the fragility an allowlist has here.
+2. **Class identity was the wrong axis entirely.** `DHLink._to_ets()` shows
+   joint-last compliance actually tracks the `mdh` flag, not the `DHRobot`
+   class: the MDH branch reorders a revolute link's `d` translation to
+   *precede* the joint rotation (valid — a z-rotation and z-translation
+   about/along the same axis commute), so the joint ET is last regardless
+   of `d`. A `DHRobot(mdh=True)` instance is therefore structurally fine
+   through `Robot.rne()`, while `mdh=False` is not — the class-name check
+   would have wrongly rejected the compliant MDH case too. The final guard
+   checks `self.mdh` directly (defaulting to `True` via `getattr` for
+   non-DHRobot types, which have no DH-convention concept and are already
+   guaranteed joint-last some other way).
+
+Verifying the MDH case numerically (nonzero `d`, `alpha`, mass, and full
+inertia tensor, compared against `rne_python()`) surfaced a real, separate
+bug it would otherwise have masked: `SpatialInertia(m=link.m, r=link.r)` in
+`Robot.rne()`'s inertia accumulation never passed `I=link.I` — the
+rotational inertia tensor was silently dropped for every link, for every
+`Robot`/`ERobot`/`URDFRobot`/`PoERobot`/MDH-`DHRobot` call, not just the MDH
+edge case. Fixed alongside this guard. Not caught earlier because the
+`TwoLink`-based `Robot.rne()`-vs-`rne_python()` equivalence tests used
+default (zero) inertia; the tests that *did* set `inertia=True` only
+compared the C path against `rne_python()`, never `Robot.rne()`.
+
+**Revisit when the class hierarchy redesign above happens.** If `DHRobot`
+stops being a `Robot` subclass (e.g. becomes `Robot[DHLink]` under the
+generic-`LinkType` proposal, or the "one Robot class, polymorphic `Link.A(q)`"
+design below is adopted), the whole question may become moot — either there's
+only one `Robot` class and the guard is unnecessary, or the DH-convention
+distinction is structurally explicit rather than a name-based/attribute-based
+runtime check.
+
 ---
 
 ## Forward-looking design: one Robot class, polymorphic Link.A(q)

tests/test_fknm_fallback.py

@@ -850,6 +850,22 @@ def test_mdh_rne_python_now_agrees(self):
             mdh_py = self.mdh.rne_python(q, z, z)
             nt.assert_array_almost_equal(mdh_py, truth, decimal=4)
 
+    def test_robot_rne_rejects_standard_dh(self):
+        """Robot.rne() (the ETS/Featherstone implementation, used by
+        ERobot/URDF/PoERobot) genuinely cannot handle a standard-DH
+        (mdh=False) DHRobot -- the joint isn't the last element of its own
+        ETS segment. Rather than silently returning a wrong torque (the
+        old behaviour -- see rne.md/tech-debt.md), it now asserts. This
+        confirms the guard fires for the one case it must, complementing
+        test_robot_rne_on_mdh_variant_matches_standard_dh_rne_python (which
+        confirms it does *not* fire, and gives correct results, for the
+        mdh=True case)."""
+        from roboticstoolbox.robot.Robot import Robot as RobotBase
+
+        z = np.zeros(2)
+        with self.assertRaises(AssertionError):
+            RobotBase.rne(self.std, self.poses[0], z, z)
+
 
 # ---------------------------------------------------------------------------
 # Actuator dynamics (Jm, G, B, Tc) and non-zero link inertia: C vs