bullet3 icon indicating copy to clipboard operation
bullet3 copied to clipboard

Nbelakovski/determinism changes

Open nbelakovski opened this issue 4 years ago • 2 comments

I've finally managed to find time to get follow up on #1577! Not sure if you're still maintaining the sections of code involved here, but at least I can submit this and now that there's a test (courtesy of @mihan69rus) I feel like it would be ok to leave this open.

To re-summarize: These changes "improve" the determinism of the serialization/deserialization mechanism. I say "improve" because I haven't looked at all features of Bullet, but for the subset covered by the test, we can say those are deterministic.

The main changes are:

  • Converting ConvexInternalShape to use both double and float, as it had been serializing double data to float, which was a major source of non-determinism. This had a downstream impact on a number of shapes that all consume ConvexInternalShape.
  • Deserializing velocity by default
  • Adding a motionstate to deserialized RigidBodies in the MultiBodyWorldImporter (in the original submission, motionstate was added by default to RigidBodies upon their construction, but there was feedback on that so I moved it to deserialize-time)
  • Sorting the contact manifolds after deserialization, as having them in an alternate order also impacted determinism.

I've updated the btSerializer and bullet.h files. For btSerializer, since I'm on a 64 bit system it insisted on generating the 64 bit version, but I simply edited the resulting file by hand to remove '64' from the variable names and things appear to work OK. I took a look at the code of makesdna before doing that to convince myself it would be valid.

Since @mihan69rus added a test, I feel like we can leave this open if you don't have time to get to it, my concern in the past was that this branch might get stale if it's simply left open and it would be tough to update it, but a unit test of course makes that much easier.

Happy to make any further changes as you deem necessary.

nbelakovski avatar May 08 '20 20:05 nbelakovski

This appears to have broken an already existing test for deterministic save restore with pybullet. I'm investigating.

nbelakovski avatar May 11 '20 20:05 nbelakovski

I made some changes that fix the failing test. It was failing because of some code I had added to sort the contact manifolds after they were deserialized, to get them into the order in which they were serialized. Although that fixed my test, it broke the pybullet one. I was able to simplify my test so that it still tests the changes I made, but doesn't break if the contact manifolds aren't sorted. I'm not sure exactly what's going on with those manifolds, but I think it's best if that problem is left for another day and this PR stays focused on the changes to allow certain shapes to be serialized as both double and float.

nbelakovski avatar May 12 '20 03:05 nbelakovski