-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
CalcSpatialInertia fails on this Hello Robot obj #21666
Comments
The
I'm guessing that the obj file in question might not meet those conditions. So there are at least three problems: (1) Instead of returning a "meaningless" value like it promised, sometimes (2) The docs for the related overload (3) Our parser(s?) call Or, if this particular obj file actually meets those requirements, then we have even more going wrong beyond those points. On first impression, the ideal fix would be to update the contract of In the meantime, possibly we should hotfix the function to at least never throw. |
Thanks Jeremy. I completely agree, and would note that I think propagating the error condition properly up through the parser (without a throw) would be better than nothing. Removing the apparently overly conservative requirements would of course be better. As a fallback, I could imagine CalcSpatialInertia could try to compute the inertia and if it is not physically valid, then it could fall back to a simpler approximation (like taking the volume of the convex hull, or even obb), so that it always returns something valid. |
P.S. Geometric analysis is still currently beyond Drake's capacity. But it's been on my wish list for years. |
As a point of reference, one of the ways that SDFormat specifies handling auto inertia for non-convex meshes is to convert the mesh into a voxelized approximation, and sum the inertia of the voxels. |
I think we already support non-convex. It's not clear that would work for open faces. |
Well, I was trying to talk about what to do with non-convex meshes that we currently mis-process (not non-convex meshes overall), but anyway that was getting off the point. Sean had a better question..
This is the only question that matters. We are writing a parser for the mujoco file format. The only question that matters is what is specified to happen, by the file format documentation, when auto-inertia is being used on an open mesh. Our job is to implement that specification. If the specification doesn't say, they we ask upstream to clarify it, and in the meantime we can guess what the spec would say by reading the mujoco parsing code and seeing what happens in their first-party parser. My attempts to reason about this bottom-up from |
To be clear - non-convexity is not an issue here. Non-convexity doesn't cause problems at all. Open meshes do. Agreed - we need to find out what mujoco is doing. 100% on board with that. |
I was treating the Convex shape as a special case because computing its uniform-density inertia should always be easy and correct, even if some triangles are missing or wound the wrong way or etc. One we take the hull, the inertia should be easy. Probably a moot point for this thread though, since I don't think the |
BTW Mujoco explicitly has a parameter called "shellinertia" indicating whether the mass is distributed on the surface. Its default value is Also, there's a compiler option, exactmeshinertia that defaults to So, having done a quick pass through the documentation, I didn't see enough information vis a vis computing the spatial inertia from a mesh. I'd invite others to see if they can find it, but I suspect we'll have to turn to the code/developers to find out. |
The mujoco ComputeVolume() code for a mesh appears to be here: https://github.com/google-deepmind/mujoco/blob/df7ea3ed3350164d0f111c12870e46bc59439a96/src/user/user_mesh.cc#L1214 . I agree that they aren't doing anything fancy there. |
My current mental model is that mujoco is probably generating physically invalid inertias for those models, but then not doing the consistency check. But that should be verified. |
in #21403, i've implemented the fallback of using the volume of the OBB if the CalcSpatialInertia fails. (with a TODO reminding me that we could do better by taking the volume of the convex hull). |
I don't think invalid inertias are generated. If there is a specific question about inertia computation (which I agree needs work!), I'm happy to address. |
Hi Yuval. Indeed, I see now here that you catch the negative volume case and use the convex hull if needed (I did the OBB to be quick, but using the convex hull as the fallback is now a to-do in our mujoco parser, too). And you do have the sanity check a bit below. Sorry for not looking more thoroughly! |
BTW: My updated understanding is that our CalcSpatialInteria (and MuJoCo's comparable method), only declare failure -- and thus will fall back to a convex hull computation -- in the event of total failure (e.g. negative volume or nonphysical inertias). I suspect that if the mesh is not watertight or has some strange windings, then we can still be arbitrarily wrong (e.g. removing volume that should be added, but not passing the threshold into negative volume?) I believe the algorithms work by taking a central point then iterating over surface triangles, computing the quat mesh volume/inertia. Without having studied it carefully, I believe that the intended behavior for this algorithm must be to sometimes compute negative volume/inertia for one step; this must be the reason that it works even for nonconvex meshes? So I don't know if there is an more narrow error condition that one could check, but it's worth keeping in mind. |
Side note, here's roughly where MuJoCo stands WRT inertia computation. At some point we added the A parallel bit of storyline is the surface inertia ( So our current plan is:
@quagla did I forget anything? If you guys have comments on the above plan, or if you can come up with the general formula for the inertia of a thick shell geom, let us know! |
In the past, I used the ray stabbing method here for computing inertia for not watertight meshes (a cuda implementation exists here). This method allows you determine whether a point is inside or outside a mesh for non-watertight meshes (I did not test ambiguous situations). One can then sample points in the object's bounding box, filter the inside points, and treat them as point masses for computing inertia with a discrete approximation. I tested it on various shapes and got almost identical results to Drake and trimesh. |
@yuvaltassa has done a great job summarizing the challenges. It's one of the reason that Drake's documentation is tenuous: "Meet the requirements or don't be guaranteed on getting something physical." Nice to know the convex hull is the fallback -- that's something I'll add today so that we're playing the same game as mujoco. |
FYI #21693 will go ahead and finish replicating the mujoco logic -- using convex hull as the fallback. At that point, I believe we can close this. |
D'oh! I spoke too soon. While #21693 will go ahead and get Drake's observable behavior in line with mujoco, it still isn't doing it the "right way" -- throwing and catching exceptions. So, this should stay open to track that part of the issue. |
FWIW, #21929 is motivating me to clean up the throwing from CalcSpatialInertia. Rather, I will work with that PR to offer some |
I dug a bit more and the phrase, "So the convex approximation has to stay." has gained new signficance. tl;dr Drake has implemented the "convex approximation" as the convex hull. Mujoco has not. Even though it prints a warning claiming the convex hull is being used, in fact, that is not the case. In ComputeVolume() the request to not use the "exact" mesh inertia causes the volume of each computed tet to become positive. The sum of all of these volume magnitudes is not generally the volume of the convex hull. It is some value greater than or equal to the convex hull's volume. The tetrahedra are calculated based on the centroid of all of the triangles (which is definitely contained within the convex hull of the mesh). However, if the mesh contains layers or voids between the surface of the convex hull and that centroid point, extra volume will be added in. Potentially a great deal of volume. For example, imagine a spherical shell with outer radius R and inner radius r. The approach for computing the volume would essentially compute the volume of the sphere of radius R and subtract the volume of a sphere with radius r (based on the winding of triangle vertices implying surface normals in opposite directions). However, when Is this a problem? Probably not a big one. Between the assumption of uniform density throughout the mesh, largely arbitrary density values, and the fact that even the exact convex hull is probably a poor approximation of the actual mesh, the mujoco volume computed here is not particularly more arbitrary than one computed on the convex hull. If there is actually a problem it is that the center of mass of this convex approximation could be offset in a silly way. If the underlying mesh had a void inside of it (like a hole bored through it), the actual center of mass would be away from the void. The center of mass of the convex hull would be closer to the void, and the center of mass computed by this approach would bias the center of mass even closer to the void. I'd still say that what Drake is doing is good, inspired by mujoco's warning. Using the convex hull as a fall back proxy is reasonable. But that doesn't mean Drake will produce the same spatial inertia as mujoco in when they both fall back. |
What happened?
Following #20444, I've put a simple failure reproduction here: 2fe7f24
This attempts to compute the spatial inertia from the hello robot model in mujoco menagerie but fails with
Additionally the
CalcSpatialInertia
doc strings do not actually declare that they could throw an exception for this reason.Version
No response
What operating system are you using?
Ubuntu 22.04
What installation option are you using?
compiled from source code using Bazel
Relevant log output
No response
The text was updated successfully, but these errors were encountered: