-
Notifications
You must be signed in to change notification settings - Fork 25
Add InverseKinematics class #128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
cc @STaliani this can be useful for you as well! |
…ethod to set custom joint limits
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
README.md
Outdated
ik.add_target("l_sole", target_type=TargetType.POSE, weight=1.0, as_constraint=False) | ||
ik.add_ball_constraint(frame_1, frame_2, as_constraint=False) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can it not a constraint if I am calling it with add_ball_constraint
? We historically had a huge confusion between constraints and tasks in iDynTree, but probably we can try to avoid it here, for example using a different term (as_optimization_constraint
or something similar?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense!
What do you think about something like ik.add_ball_constraint(frame_1, frame_2, as_soft_constraint=True)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, but probably we may want to have an argument with a similar name in add_target
to avoid confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could add to the add_target
method an input called as_soft_constraint
as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to
ik.add_target("l_sole", target_type=TargetType.POSE, as_soft_constraint=True, weight=1.0)
ik.add_ball_constraint(frame_1, frame_2, as_soft_constraint=True)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a new inverse kinematics solver to the adam.casadi
package, enabling users to define and solve IK problems with position, orientation, pose targets, and frame‐to‐frame constraints.
- Introduces
InverseKinematics
class ininverse_kinematics.py
with methods for adding targets, constraints, setting initial guesses, and solving via CasADi. - Updates package exports to include the new class and its
TargetType
. - Adjusts CI and packaging to add the new
liecasadi
dependency. - Expands
README.md
with installation tweaks and an IK usage example.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
src/adam/casadi/inverse_kinematics.py | New IK solver implementation with targets & constraints |
src/adam/casadi/init.py | Export InverseKinematics and TargetType |
pyproject.toml | Adds liecasadi to test dependencies |
ci_env_win.yml & ci_env.yml | Includes liecasadi in CI environment |
README.md | Updates installation commands and adds IK usage docs |
Comments suppressed due to low confidence (2)
src/adam/casadi/inverse_kinematics.py:243
- [nitpick] The parameter
as_constraint
here is inconsistent withas_soft_constraint
used byadd_frames_constraint
; consider renaming toas_soft_constraint
or harmonizing naming for clarity.
def add_fixed_constraint(
pyproject.toml:51
- The
liecasadi
dependency is only added under the test extra; sinceinverse_kinematics.py
imports it at runtime, it should be included in the main or 'all' dependencies to avoid missing package errors.
"liecasadi",
if len(frames_list) < 2: | ||
raise ValueError("At least two frames are required for distance constraint") | ||
for i in range(len(frames_list) - 1): | ||
print( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Avoid using print
for diagnostic messages in library code; consider using a logger or removing this statement.
print( | |
logger.debug( |
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
): | ||
"""Add a target position for the IK solver. | ||
|
||
Args: | ||
frame (str): The name of the frame to target. | ||
as_soft_constraint (bool, optional): If True, adds the target as a cost term instead of a constraint. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fear this sentence is misleading. "If True, adds the target as a cost term instead of a constraint." seems to indicate this parameter is False
by default, while it is actually True by default, perhaps this can be made more clear?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally true! Updated all the args descriptions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments.
Co-authored-by: Silvio Traversaro <[email protected]>
Co-authored-by: Silvio Traversaro <[email protected]>
Thanks a lot @traversaro! Addressed the comments. |
This pull request introduces an inverse kinematics module to the
adam.casadi
package. The changes include the addition of a newInverseKinematics
class, which provides functionality for solving various inverse kinematics problems, such as position, orientation, and pose targets, as well as constraints between frames. The most important changes are detailed below.New Module Addition
src/adam/casadi/inverse_kinematics.py
: Added theInverseKinematics
class, which includes methods for defining targets (position, orientation, pose), updating targets, adding constraints (e.g., ball, hinge, fixed), setting initial guesses, solving the optimization problem, and retrieving the solution. This class leverages CasADi for optimization and symbolic computation.Package Integration
src/adam/casadi/__init__.py
: Imported the newInverseKinematics
module into theadam.casadi
package, making it accessible as part of the package's API.📚 Documentation preview 📚: https://adam-docs--128.org.readthedocs.build/en/128/