facebookresearch / polymetis

Write PyTorch controllers, test them in simulation, and seamlessly transfer to real-time hardware.
MIT License
52 stars 2 forks source link

Fix timestamp #3

Closed exhaustin closed 3 years ago

exhaustin commented 3 years ago

Naively converting the timestamp to a float would result in loss of precision, causing time differences to be incorrectly calculated.

Changes proposed:

Changed the tensor representation of a timestamp to a int32 tensor of size 2, with the first element representing seconds and the second representing nanoseconds.

Test plan:

Tested on a custom controller that uses the timestamp

Potential issue

1heart commented 3 years ago

Why not use a double? Also, let's add a test that catches this particular issue.

exhaustin commented 3 years ago

Why not use a double? Also, let's add a test that catches this particular issue.

Timestamps represent some notion of a absolute point in time and thus are extremely large integer values, while in most of our applications we care about differences in time in the scale of milliseconds. Float64 is able to represent around 16 significant decimal digits, which might still not be sufficient. Ideally we should obtain the exact time difference by subtracting the seconds and nanoseconds then converting that result to float, which should then be easily represented. This is something I realized only after writing the circular motion controllers.