Fix RCM margin, M3 joint limits, and add finite-difference tests - #14
Merged
Conversation
Address code-analysis items 1, 2 and 4, and add a test suite. 1. RCM constraint margin (icra2019_controller.py): the safe-radius margin returned by get_rcm_constraint() was dropped when the RCM row was stacked into the full QP (the partial (1,) right-hand side was replaced by a zeroed (1,) vector), degrading the constraint to J_rcm @ q_dot <= 0. Carry the margin into the full right-hand side so the VFI bound is applied. 2. M3 default joint limits (M3_SerialManipulatorSimulatorFriendly.cpp): the constructor only inherited the base class resize, leaving lower/upper_q_limit_ as uninitialized Eigen memory. Initialize them to a wide default (+-10 rad) so get_*_q_limit() never returns garbage and the joint-limit constraints stay feasible until real limits are set. 4. example_plot (example_load_from_file.py): read the "radius" key the loader actually produces instead of the missing "diameter" key (KeyError). Also fix example_load_from_file.main(): it passed 2-tuples to rcm_constraints (the controller unpacks (position, radius, joint_index)) and never set joint limits, so the QP was infeasible from the start. Apply the documented joint limits and use the correct 3-tuples. Tests (pure-Python dqrobotics; no _core build required, with a graceful mock fallback when the compiled extension is unavailable): - tests/test_finite_difference.py validates, by central finite differences, that each distance/plane task Jacobian differentiates the quantity its name and sign convention imply (squared distance for point/line constraints, signed plain distance for plane constraints), plus the haminus8 tool-frame Jacobian transform and the RCM margin cross-check. - tests/test_regressions.py guards the three fixes above. Co-authored-by: openhands <openhands@all-hands.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses code-analysis items 1, 2, and 4, and adds a finite-difference test suite.
This PR was created by an AI agent (OpenHands) on behalf of the user.
Bug fixes
RCM constraint margin dropped (
icra2019_controller.py). The safe-radius margin returned byget_rcm_constraint()was discarded when the RCM row was stacked into the full QP: the partial(1,)right-hand side was replaced by a zeroed(1,)vector, so the constraint degraded toJ_rcm @ q_dot <= 0and lost its safe-radius (VFI) bound. The margin is now carried into the full right-hand side (w_c_full), so the QP rhs matchesget_rcm_constraint. Verified end-to-end: the stacked RCM rhs now equals the VFI margin (e.g.-1.55,-1.46) and the QP solves.M3joint limits uninitialized (src/M3_SerialManipulatorSimulatorFriendly.cpp). The base class onlyresizeslower_q_limit_/upper_q_limit_, so the constructor left them as uninitialized Eigen memory (garbage values, non-deterministic). They are now initialized to a wide default (±10 rad) soget_*_q_limit()never returns garbage and the joint-limit constraints stay feasible until the caller sets real limits. Verified by a clean C++ rebuild: defaults are finite and ordered.example_plotKeyError(example_load_from_file.py).example_plotread the missing"diameter"key; the loader only produces"radius". It now reads"radius".Also fixed (required for the example to run)
example_load_from_file.main()passed 2-tuples torcm_constraints(the controller unpacks(position, radius, joint_index)) and never set joint limits, so it crashed / built an infeasible QP. It now applies the documented joint limits and uses the correct 3-tuples (frame index 6, matchingexample_create_needle_controller).Tests (new)
Runs with pure-Python
dqrobotics; no_corebuild is required.tests/conftest.pybridges the installed_core/marinholab.solversinto the repo package when running from the source checkout, and falls back to a mock_corewhen no compiled extension is available.tests/test_finite_difference.py— validates, by central finite differences, that each distance/plane task Jacobian differentiates the quantity its name and sign convention imply (squared distance for point/line constraints; signed plain distance for plane constraints), plus thehaminus8tool-frame Jacobian transform and an RCM-margin geometry cross-check.tests/test_regressions.py— guards the three fixes above (RCM margin in the QP rhs,M3default limits finite/ordered,example_plotreadingradius).All 12 tests pass and
pyrightremains clean (0 errors).Co-authored-by: openhands <openhands@all-hands.dev>