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

fix quaternion's gradients in PoseInverse, and a few other warp kernels #393

Merged
merged 1 commit into from
Nov 17, 2024

Conversation

YuhengZhi
Copy link
Contributor

@YuhengZhi YuhengZhi commented Sep 27, 2024

  1. The current conversions from wp.quaternion to wp.vec4 would stop gradients from properly propagating. This has caused the issue mentioned in curobo.geom.transform.pose_inverse or Pose.inverse() has incorrect gradients for quaternion #392.

    Instead of

    out_v = wp.vec4()
    out_v[0] = out_q[3]
    out_v[1] = out_q[0]
    out_v[2] = out_q[1]
    out_v[3] = out_q[2]
    

    , this commit does the conversion from quaterniom to vec4 as

    out_v = wp.vec4(out_q[3], out_q[0], out_q[1], out_q[2])
    

    which seems to produce correct results in the example used in curobo.geom.transform.pose_inverse or Pose.inverse() has incorrect gradients for quaternion #392 .

  2. A few other wp.kernel's in the same file were using the same conversion method, so are all modified accordingly.

  3. Fixed missing/extra None's in a few backward functions, mentioned in Incorrect number of gradients in backward functions in curobo/src/curobo/geom/transform.py #382 .

Please feel free to let me know if there are any issues.

@balakumar-s
Copy link
Collaborator

Thanks for the fix!

@balakumar-s balakumar-s merged commit a1c1106 into NVlabs:main Nov 17, 2024
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