Skip to content

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

Merged
merged 21 commits into from
Jul 9, 2025
Merged

Add InverseKinematics class #128

merged 21 commits into from
Jul 9, 2025

Conversation

Giulero
Copy link
Collaborator

@Giulero Giulero commented Jul 7, 2025

This pull request introduces an inverse kinematics module to the adam.casadi package. The changes include the addition of a new InverseKinematics 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 the InverseKinematics 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 new InverseKinematics module into the adam.casadi package, making it accessible as part of the package's API.

📚 Documentation preview 📚: https://adam-docs--128.org.readthedocs.build/en/128/

@Giulero Giulero requested a review from Copilot July 7, 2025 17:56
Copilot

This comment was marked as outdated.

@traversaro
Copy link
Contributor

cc @STaliani this can be useful for you as well!

@Giulero Giulero requested a review from Copilot July 8, 2025 10:20
Copilot

This comment was marked as outdated.

@Giulero Giulero requested a review from Copilot July 8, 2025 11:15
Copilot

This comment was marked as outdated.

@Giulero Giulero self-assigned this Jul 8, 2025
@Giulero Giulero requested a review from Copilot July 8, 2025 11:17
Copilot

This comment was marked as outdated.

README.md Outdated
Comment on lines 377 to 378
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)
Copy link
Contributor

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?).

Copy link
Collaborator Author

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)?

Copy link
Contributor

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.

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

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)

@Giulero Giulero requested a review from Copilot July 9, 2025 07:47
Copy link
Contributor

@Copilot Copilot AI left a 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 in inverse_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 with as_soft_constraint used by add_frames_constraint; consider renaming to as_soft_constraint or harmonizing naming for clarity.
    def add_fixed_constraint(

pyproject.toml:51

  • The liecasadi dependency is only added under the test extra; since inverse_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(
Copy link
Preview

Copilot AI Jul 9, 2025

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.

Suggested change
print(
logger.debug(

Copilot uses AI. Check for mistakes.

):
"""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.
Copy link
Contributor

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?

Copy link
Collaborator Author

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

Copy link
Contributor

@traversaro traversaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comments.

@Giulero
Copy link
Collaborator Author

Giulero commented Jul 9, 2025

Thanks a lot @traversaro! Addressed the comments.

@Giulero Giulero merged commit 0d3e943 into main Jul 9, 2025
17 of 18 checks passed
@GiulioRomualdi GiulioRomualdi deleted the ik branch July 9, 2025 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants