Fixes details in documentation for Pink IK Null-space posture task#4524
Fixes details in documentation for Pink IK Null-space posture task#4524michaellin6 wants to merge 3 commits intoisaac-sim:developfrom
Conversation
Greptile OverviewGreptile SummaryThis PR addresses issue #4417 by fixing documentation and adding configurability to the Pink IK controller. The changes include:
Critical Issue Found:
This inconsistency needs to be resolved by either reverting the documentation changes or updating the implementation to match. Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant PinkIKController
participant Config as PinkIKControllerCfg
participant Solver as Pink solve_ik
participant Task as NullSpacePostureTask
User->>Config: Set qp_solver (e.g., "daqp", "quadprog")
User->>PinkIKController: Initialize with config
PinkIKController->>Task: Set up NullSpacePostureTask
Task->>Task: Initialize with controlled_frames & controlled_joints
User->>PinkIKController: compute(curr_joint_pos, dt)
PinkIKController->>Task: compute_error(configuration)
Task->>Task: pin.difference(target_q, current_q)
Note over Task: Computes (q* - q) [Implementation]<br/>Docs say (q - q*) [Inconsistency]
Task-->>PinkIKController: Error vector
PinkIKController->>Task: compute_jacobian(configuration)
Task->>Task: Build null space projector N = I - J+ @ J
Task-->>PinkIKController: Null space Jacobian
PinkIKController->>Solver: solve_ik(config, tasks, dt, solver=cfg.qp_solver)
Note over Solver: Uses configurable solver<br/>(previously hardcoded "daqp")
Solver-->>PinkIKController: Joint velocities
PinkIKController-->>User: Target joint positions
|
| .. math:: | ||
|
|
||
| \mathbf{e}(\mathbf{q}) = \mathbf{M} \cdot (\mathbf{q}^* - \mathbf{q}) | ||
| \mathbf{e}(\mathbf{q}) = \mathbf{M} \cdot (\mathbf{q} - \mathbf{q}^*) |
There was a problem hiding this comment.
The documentation now states the formula is (q - q*) but the actual implementation at line 200 uses pin.difference(configuration.model, self.target_q, configuration.q) which computes (q* - q). Either the documentation fix is incorrect, or the implementation needs to be updated to match.
Same issue at line 75 and in the docstring at line 176.
Additional Comments (1)
|
|
@kellyguo11 Should this target develop instead? It doesn't seem to be fixing a bug, so I don't see the need to target main. |
…a tests for NullSpacePosture Task. Making Pink QP solver a parameter in config.
95c26e0 to
06278e5
Compare
|
@michaellin6 the pink IK test is still failing, is this expected with this PR? It has been spotty for me, sometimes passing, others failing. |
this is likely due to the issue fixed by this PR: #4595 We can rebase once that is merged in and hopefully tests will pass then. |
Description
Fixes typo in NullSpacePostureTask documentation. Adds additional tests for NullSpacePostureTask. Makes QP solver for Pink a parameter in config file so users can choose other solver options, but still defaults to 'daqp'.
Fixes # (issue)
Fixes issues raised by community user: #4417
Type of change
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there