IsaacLab icon indicating copy to clipboard operation
IsaacLab copied to clipboard

Adds `IMU` sensor

Open pascal-roth opened this issue 1 year ago • 16 comments

Description

Add IMU sensor with cfg class IMUCfg and data class IMUData. Compared to the Isaac Sim implementation of the IMU Sensor, this sensor directly accesses the PhysX view buffers for speed acceleration.

Fixes #440

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist

  • [x] I have run the pre-commit checks with ./isaaclab.sh --format
  • [x] I have made corresponding changes to the documentation
  • [x] My changes generate no new warnings
  • [x] I have added tests that prove my fix is effective or that my feature works
  • [x] I have updated the changelog and the corresponding version in the extension's config/extension.toml file
  • [x] I have added my name to the CONTRIBUTORS.md or my name already exists there

pascal-roth avatar Jul 02 '24 15:07 pascal-roth

Hi @pascal-roth - we have a potential early adopter of this sensor at the institute. Would you say it's ready to go or still needs to address some of the comments?

Thanks!

jsmith-bdai avatar Jul 24 '24 19:07 jsmith-bdai

@pascal-roth

In my opinion, names should be ImuCfg, ImuData, etc. rather than IMUCgf, etc...

Toni-SM avatar Jul 24 '24 23:07 Toni-SM

Hi @pascal-roth - we have a potential early adopter of this sensor at the institute. Would you say it's ready to go or still needs to address some of the comments?

Thanks!

The sensor should work if no offset is given to it. For the case where an offset is present, I am not 100% sure of my math. Especially as the test is not working yet for it. If the early adapter can spot any mistakes, I would be really grateful.

pascal-roth avatar Jul 25 '24 13:07 pascal-roth

@Mayankm96 and @Dhoeller19, I have an issue with the indented test of the IMU. I want to compare the output with the IMU Sensor of Isaac Sim for a single robot, but it just shows gravity acceleration and does not detect the robot's motion. It is located under the robot prim, and I switched off fabric. In addition, I am getting the following error:

[Error] [omni.physx.tensors.plugin] Incompatible device of velocity tensor in function getVelocities: expected device 0, received device -1

Any idea what goes wrong here? The code is located in the test_imu.py file.

pascal-roth avatar Jul 25 '24 13:07 pascal-roth

@pascal-roth we're happy to help take this PR over if you don't have the bandwidth to work on it. Don't want to steal your baby :laughing: - only offering in case

@jtigue-bdai will be doing some IMU work here at the institute so please let us know and you can do a handover (or discuss future improvements) with him if that helps ya out

jsmith-bdai avatar Aug 09 '24 15:08 jsmith-bdai

@Mayankm96 I fixed the test. Currently, we are still failing one, and I am not sure why. All the quantities except the lin_acc_w are correct. This calculated lin_vel_w with the offset has an error of 1e-3, which becomes large when divided by the timestep. IMO, the error is too large for a floating point error, but I checked the math now again, and it should be correct. Is it possible that the phyx value can be off by that amount?

pascal-roth avatar Aug 09 '24 15:08 pascal-roth

@jsmith-bdai, everything has been ready for a while. It is literally just the offset computation for the lin_acc_w that has errors, and I am not sure why. Feel free to check the math.

pascal-roth avatar Aug 09 '24 15:08 pascal-roth

@pascal-roth we have been playing around with it here. I poked everywhere I could think in terms of the math and that all seems correct. After that we started to look at the test it self. We noticed a few things:

  1. the test_offset_calculations starts with the anymal penetrating the ground. We removed the ground and that seemed to help but not completely.
  2. Then we ignored the first 10 iterations for the test if idx <10: then continue. During a free fall test this allowed linear velocity error atol/rtol to be 1e-3
  3. If we let it free fall the linear velocity seems to stay within tolerance, but the linear acceleration has more trouble. But if we add contact back in (i.e. drop the anymal from higher ant let it hit the gorund) it doesn't like the contact
  4. We also checked to make sure the angular velocity w.r.t the world of the two frames (on the same body) was equal. This checks out.
  5. We also tried CPU vs GPU and they both have similar issues.

Still looking through a few things, but wanted to let you know what we found.

jtigue-bdai avatar Aug 12 '24 12:08 jtigue-bdai

@pascal-roth nice did the get_accelerations come with the new Isaac Sim release?

Also I found an issue with why the previous method wasn't working. The get_velocities returns the velocity at the COM of the RigidBody. Thats why when you used the offset it wouldn't match. Also this means if we want the velocity at the Link frame we will need to transform it back to the link frame by the com.

I have a feeling the get_accelerations will also be at the com of the body rather than the link frame. I created a simple 1 joint urdf test that can be used to calculate the ground truth. feature/imu_com_offset

jtigue-bdai avatar Aug 13 '24 12:08 jtigue-bdai

@jsmith-bdai and @jtigue-bdai we investigated the issue further and tried different solvers. By now, we are pretty convinced that it has to do with Pysx. Raised a new issue which will be forwarded to the PhysX team

pascal-roth avatar Aug 13 '24 12:08 pascal-roth

the main issue for me is currently still the acceleration errors; even the angular acceleration is really off which then also negatively effects the linear accelerations.

In my understanding, the angular acceleration is absolut for a rigid body so there should be no difference.

pascal-roth avatar Aug 15 '24 15:08 pascal-roth

the main issue for me is currently still the acceleration errors; even the angular acceleration is really off which then also negatively effects the linear accelerations.

In my understanding, the angular acceleration is absolut for a rigid body so there should be no difference.

Interesting I haven't see too much issue with the angular acceleration. But yeah i agree it should be equivalent for frames on the same body. Is that using the anymal based test?

jtigue-bdai avatar Aug 15 '24 16:08 jtigue-bdai

yes, if you run the default anymal test now, the angular velocities are as follows:

# ang_acc_w imu_link
tensor([[-2.4121, -5.4381, -0.0945],
        [-2.3416, -5.4702, -0.1113]], device='cuda:0')
# ang_acc_w base
tensor([[-1.0775e+01, -6.5843e+00, -9.3957e-03],
        [-1.0707e+01, -6.6164e+00, -2.6417e-02]], device='cuda:0')

which then also impacts the lin_acc_w

pascal-roth avatar Aug 16 '24 07:08 pascal-roth

Hmm yeah those were much closer in my basic pendulum. In the anymal test does the robot start penetrating the ground? I noticed it was doing that when I first started looking. Maybe try removing the ground plane and see if you get the same result?

jtigue-bdai avatar Aug 16 '24 11:08 jtigue-bdai

@jtigue-bdai added some more warning; everything else is unchanged from your last workover. Thanks a lot for the changes there, it should be good to go now. Still, we should consider switching to the PhysX accelerations once all issues are solved from their side.

@Mayankm96 can we get the sensor merged before the new release?

pascal-roth avatar Sep 19 '24 19:09 pascal-roth

@Mayankm96, ok all changes were implemented. Looks like some CI isn't passing but I don't have access to diagnose.

jtigue-bdai avatar Oct 04 '24 21:10 jtigue-bdai

Could you please review and merge this pull request at your earliest convenience? I need it for my project ASAP.

ammousa avatar Oct 12 '24 12:10 ammousa