Skip to content
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

Inverse kinematics #29

Merged
merged 33 commits into from
Feb 16, 2024
Merged

Inverse kinematics #29

merged 33 commits into from
Feb 16, 2024

Conversation

LemonPi
Copy link
Member

@LemonPi LemonPi commented Nov 27, 2023

Add inverse kinematics (IK) via basic pseudo inverse method. Comes with many options for early termination and so on.

Copy link
Contributor

@PeterMitrano PeterMitrano left a comment

Choose a reason for hiding this comment

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

Are we trying to support multi-ee IK yet? I assume not since we don't have that jacobian method implemented but just want to confirm.

Also I modified the test so check IK problems generated by FK and the results suggest it's not very reliable in finding a solution:

Convergence: 517 / 10000
Iterations 30
Success 257 / 1000 goals

src/pytorch_kinematics/ik.py Outdated Show resolved Hide resolved
tests/test_inverse_kinematics.py Outdated Show resolved Hide resolved
tests/test_inverse_kinematics.py Outdated Show resolved Hide resolved
tests/test_inverse_kinematics.py Outdated Show resolved Hide resolved
"""
pos_diff = target_pos.unsqueeze(1) - m[:, :, :3, 3]
pos_diff = pos_diff.view(-1, 3, 1)
rot_diff = target_rot_rpy.unsqueeze(1) - rotation_conversions.matrix_to_euler_angles(m[:, :, :3, :3],
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we sure this is a good rotation distance? Just substracting euler angles isn't generally a good idea -- you have potential wrap-around/discontinuity issues

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for finding this! I replaced it with unit quaternion error (the quaternion required to get from current to desired orientation), converted to an axis angle representation.

Note that this is for computing error in end effector position and orientation to be multiplied by the jacobian pseudoinverse, so it needs to be 6D. [\dot{x} \omega] = J \dot{q} where we are approximating it to [\Delta{x} \Delta{r}] = J \Delta{q}

It's getting much better convergence now.

@LemonPi
Copy link
Member Author

LemonPi commented Feb 14, 2024

mujoco_menagerie seems to need jax (workflow file)

      - name: Clone mujoco_menagerie repository into the tests/ folder
        run: |
          git clone https://github.com/google-deepmind/mujoco_menagerie
        working-directory: ${{ runner.workspace }}/pytorch_kinematics/tests

how do you think we should include it? Something like:

      - name: Clone mujoco_menagerie repository into the tests/ folder
        run: |
          python -m pip install "jax[cpu]"
          git clone https://github.com/google-deepmind/mujoco_menagerie
        working-directory: ${{ runner.workspace }}/pytorch_kinematics/tests

Copy link
Contributor

@PeterMitrano PeterMitrano left a comment

Choose a reason for hiding this comment

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

just one small comment, otherwise LGTM

pyproject.toml Outdated
test = [
"pytest",
"pybullet",
"jax",
Copy link
Contributor

Choose a reason for hiding this comment

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

do we actually need this? I don't see it being used

Copy link
Member Author

Choose a reason for hiding this comment

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

This has been removed a couple commits back (only needed for mujoco menagerie)

@LemonPi LemonPi merged commit 42645b7 into master Feb 16, 2024
3 checks passed
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.

2 participants