Repository navigation
Support Mink 1.4 - #35
Merged
Merged
Conversation
Mink 1.4.0 removed the private Task._assemble_qp (kevinzakka/mink#189), which BoundedFrameTask and NullspacePostureTask used in their compute_qp_objective() overrides. Since Mink 1.2.0, solve_ik() builds the objective from compute_qp_residual() and doesn't call compute_qp_objective() for tasks that provide a residual. Both tasks already implement compute_qp_residual(), so drop the compute_qp_objective() overrides. Since Mink 1.4.0, Task.compute_qp_objective() is derived from compute_qp_residual(). Tests now compare residuals instead of objectives. They build the expected error with the same _error_and_jacobian() path as the implementation, because Mink 1.4.0's native compute_error() and _error_and_jacobian() can differ in the last few bits. Also add Mink 1.3.0 to the CI matrix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Removing the override causes direct compute_qp_objective() calls on supported Mink versions to bypass position and orientation limits, requiring a compatibility override and bounded-objective regression coverage.
1 open issue
What changed in this PR
Updates the QP task wrappers for Mink 1.4, which removed the private _assemble_qp helper.
- Removes objective overrides and unused identity-matrix caches from both task wrappers.
- Updates tests to compare residuals and use Mink’s combined error/Jacobian path.
- Adds Mink 1.3.0 to the CI matrix alongside 1.2.0 and the latest release.
| File | Description |
|---|---|
tests/test_qp_tasks.py |
Replaces private objective assembly with residual-based checks. |
src/openarm_control/qp/nullspace_posture_task.py |
Removes the objective override and identity cache. |
src/openarm_control/qp/bounded_frame_task.py |
Removes the objective override and identity cache. |
.github/workflows/test.yaml |
Adds Mink 1.3.0 compatibility testing. |
Review effort: Balanced
| @@ -199,21 +198,3 @@ def _bounded_terms( | |||
| error[:3] - full_error[:3] | |||
| ) | |||
| return error, jacobian | |||
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.
Mink 1.4.0 removed the private Task._assemble_qp
(kevinzakka/mink#189), which BoundedFrameTask and
NullspacePostureTask used in their compute_qp_objective() overrides.
Since Mink 1.2.0, solve_ik() builds the objective from compute_qp_residual() and doesn't call compute_qp_objective() for tasks that provide a residual. Both tasks already implement compute_qp_residual(), so drop the compute_qp_objective() overrides. Since Mink 1.4.0, Task.compute_qp_objective() is derived from compute_qp_residual().
Tests now compare residuals instead of objectives. They build the expected error with the same _error_and_jacobian() path as the implementation, because Mink 1.4.0's native compute_error() and _error_and_jacobian() can differ in the last few bits.
Also add Mink 1.3.0 to the CI matrix.