Skip to content

Commit 2f43a3c

Browse files
petercorkeclaude
andauthored
fix(ik): _random_q() silently produces garbage for non-finite joint limits (#648)
* fix(ik): _random_q() silently produces garbage for non-finite joint limits _random_q() (used by every numeric IK solver -- IK_NR/IK_GN/IK_LM/IK_QP -- to seed random restarts) sampled directly from a joint's qlim with no check that the limits were actually finite. A joint with a bad (non-finite, e.g. inf/-inf) limit baked into its model's own data -- this was the real root cause behind #485's KinovaGen3 report, whose own qlim was fixed separately without ever patching this gap -- caused either a silent NaN joint value or an opaque internal numpy error (OverflowError: high - low range exceeds valid bounds, depending on which RNG code path is hit), instead of a clear diagnostic pointing at the actual bad joint. Now raises a ValueError naming the offending joint index(es) and their bad qlim before sampling, matching the existing convention elsewhere in this code (e.g. ETS.qlim already raises for an unset prismatic limit) of failing loudly rather than propagating a silent bad value. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(ik): apply the same non-finite-qlim guard to the C++ IK fast path ik.py's _random_q() (fixed in the previous commit) and ik.cpp's own _rand_q() are separate implementations reached by different public entry points -- ikine_LM/ikine_NR/ikine_GN/ikine_QP go through the pure-Python IK_LM/IK_NR/IK_GN/IK_QP classes, while ik_LM/ik_NR/ik_GN (documented as "a fast solver implemented in C++") go through ik.cpp via nanobind. The C++ side had the identical gap with no guard at all: sampling a non-finite qlim via raw Eigen arithmetic silently produced a NaN q, which the solve loop then burned through every one of its random restarts on before returning a "failed" solution containing NaN -- no exception, no diagnostic. Since RTB's public API is "IK" regardless of which implementation backs a given method name, both solvers now enforce the same contract: _rand_q() throws nb::value_error (mapping to a Python ValueError, matching the Python-side message) before sampling if any joint's qlim is non-finite. Verified by rebuilding the compiled extension locally and confirming the fail-then-pass behavior directly: pre-fix, ets.ik_LM() silently returned (q=[nan], success=0) after 101 wasted searches; post-fix, it raises ValueError immediately. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 8caa363 commit 2f43a3c

3 files changed

Lines changed: 63 additions & 4 deletions

File tree

src/roboticstoolbox/ets/cpp-extensions/ik.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,13 @@
77

88
#include <Python.h>
99
#include <math.h>
10+
#include <cmath>
1011
#include <iostream>
1112
#include <functional>
1213
#include <Eigen/Dense>
14+
#include <nanobind/nanobind.h>
15+
16+
namespace nb = nanobind;
1317

1418
// ---------------------------------------------------------------------------
1519
// Shared loop kernel — all five IK solvers use this.
@@ -290,6 +294,19 @@ extern "C"
290294
Eigen::Map<Eigen::ArrayXd> qlim_l(ets->qlim_l, ets->n);
291295
Eigen::Map<Eigen::ArrayXd> q_range2(ets->q_range2, ets->n);
292296

297+
// A joint with a non-finite limit (inf/-inf/NaN, typically a bad
298+
// value baked into a robot model's own joint-limit data) would
299+
// otherwise silently propagate inf/NaN into q below, with no
300+
// diagnostic at all -- mirrors the equivalent check in the
301+
// pure-Python solver path (IK.py's _random_q()).
302+
for (int i = 0; i < ets->n; i++)
303+
{
304+
if (!std::isfinite(qlim_l(i)) || !std::isfinite(q_range2(i)))
305+
throw nb::value_error(
306+
"Joint limit(s) are not finite -- can't generate a "
307+
"random configuration within an infinite/undefined range.");
308+
}
309+
293310
q = VectorX::Random(ets->n);
294311

295312
q = (q.array() + 1) * q_range2;

src/roboticstoolbox/robot/IK.py

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -406,24 +406,35 @@ def _random_q(self, ets: "rtb.ETS", i: int = 1) -> np.ndarray:
406406
:returns: An ``i x n`` ndarray of random valid joint configurations, where n
407407
is the number of joints in the ``ets``
408408
:rtype: numpy.ndarray
409+
:raises ValueError: a joint's qlim is not finite (e.g. ``inf``/``-inf``
410+
or ``NaN``, typically from a bad value in a robot model's own
411+
joint-limit data)
409412
410413
Generates a random q vector within the joint limits defined by ``ets.qlim``.
411414
"""
412415

416+
qlim = ets.qlim
417+
418+
if not np.all(np.isfinite(qlim)):
419+
bad = np.flatnonzero(~np.all(np.isfinite(qlim), axis=0))
420+
raise ValueError(
421+
f"Joint limit(s) for joint index(es) {bad.tolist()} are not "
422+
f"finite (qlim={qlim[:, bad].tolist()}) -- can't generate a "
423+
"random configuration within an infinite/undefined range."
424+
)
425+
413426
if i == 1:
414427
q = np.zeros((1, ets.n))
415428

416429
for i in range(ets.n):
417-
q[0, i] = self._private_random.uniform(ets.qlim[0, i], ets.qlim[1, i])
430+
q[0, i] = self._private_random.uniform(qlim[0, i], qlim[1, i])
418431

419432
else:
420433
q = np.zeros((i, ets.n))
421434

422435
for j in range(i):
423436
for i in range(ets.n):
424-
q[j, i] = self._private_random.uniform(
425-
ets.qlim[0, i], ets.qlim[1, i]
426-
)
437+
q[j, i] = self._private_random.uniform(qlim[0, i], qlim[1, i])
427438

428439
return q
429440

tests/test_IK.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -846,6 +846,37 @@ def test_iter_iksol(self):
846846
self.assertEqual(e, 0.1)
847847
self.assertEqual(f, "")
848848

849+
def test_random_q_rejects_non_finite_qlim(self):
850+
# _random_q() used to sample straight from ets.qlim with no check --
851+
# a joint with a bad (non-finite) limit baked into its model data
852+
# would silently produce garbage (NaN, or an opaque numpy internal
853+
# error) instead of a clear diagnostic. A finite joint's random_q
854+
# should be unaffected.
855+
et = rtb.ET.Rz(qlim=[-np.inf, np.inf])
856+
ets = rtb.ETS([et])
857+
solver = rtb.IK_LM()
858+
859+
with self.assertRaises(ValueError):
860+
solver._random_q(ets, 1)
861+
862+
good_et = rtb.ET.Rz(qlim=[-np.pi, np.pi])
863+
good_ets = rtb.ETS([good_et])
864+
q = solver._random_q(good_ets, 5)
865+
self.assertTrue(np.all(np.isfinite(q)))
866+
self.assertEqual(q.shape, (5, 1))
867+
868+
def test_ik_lm_c_rejects_non_finite_qlim(self):
869+
# Same guard, mirrored in the compiled fast-path solver (ets.ik_LM(),
870+
# backed by ik.cpp's own _rand_q()) -- this is a genuinely separate
871+
# implementation from IK_LM/_random_q() above, and used to silently
872+
# return a NaN "solution" (success=0) after burning through every
873+
# random restart, rather than raising.
874+
et = rtb.ET.Rz(qlim=[-np.inf, np.inf])
875+
ets = rtb.ETS([et])
876+
877+
with self.assertRaises(ValueError):
878+
ets.ik_LM(np.eye(4))
879+
849880

850881
if __name__ == "__main__":
851882
unittest.main()

0 commit comments

Comments
 (0)