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

//inertial/@auto: allow specifying mass instead of density #1482

Open
scpeters opened this issue Dec 18, 2023 · 11 comments
Open

//inertial/@auto: allow specifying mass instead of density #1482

scpeters opened this issue Dec 18, 2023 · 11 comments
Assignees
Labels
enhancement New feature or request

Comments

@scpeters
Copy link
Member

Desired behavior

Currently the //inertial/@auto attribute enables automatic computation of inertial properties assuming uniform density based on the shape and a density parameter. Since it can be more straightforward to measure mass by weighing an object than measuring density, center of mass, and moment of inertia, we should consider allowing the user to specify the mass and auto-compute the remaining inertial properties without using the density parameter.

Alternatives considered

Implementation suggestion

Currently automatic inertial calculations can be enabled by setting //inertial/@auto == 'true' and specifying collision geometries in a link:

<link name="link_with_collision_density">
  <inertial auto="true"/>
  <collision name="collision1">
    <density>...</density>
    <geometry>...</geometry>
  </collision>
  <collision name="collision2">
    <density>...</density>
    <geometry>...</geometry>
  </collision>
</link>

Note that when #1335 is merged, density may also be specified in the //inertial instead of //collision (see the pull request for details on precedence if it is specified in both places):

<link name="link_with_inertial_density">
  <inertial auto="true">
    <density>...</density>
  </inertial>
  <collision name="collision1">
    <geometry>...</geometry>
  </collision>
  <collision name="collision2">
    <geometry>...</geometry>
  </collision>
</link>

If a user sets //inertial/@auto == 'true' and also specifies mass, inertial pose, or moment of inertia values, a warning is thrown and the user specified values are not used.

<link name="link_with_ignored_user_inertial_values">
  <inertial auto="true">
    <mass>...</mass>
    <pose>...</pose>
    <inertia>...</inertia>
  </inertial>
  <collision name="collision1">
    <geometry>...</geometry>
  </collision>
  <collision name="collision2">
    <geometry>...</geometry>
  </collision>
</link>

This feature request is proposing that if a user sets //inertial/@auto == 'true' and specifies //inertial/mass, that the density values should be ignored, and the inertial values calculated according to the density that would result in the user-specified mass:

<link name="link_with_auto_inertial_from_mass_not_density">
  <inertial auto="true">
    <mass>...</mass>
  </inertial>
  <collision name="collision1">
    <geometry>...</geometry>
  </collision>
  <collision name="collision2">
    <geometry>...</geometry>
  </collision>
</link>

Additional context

@scpeters scpeters added the enhancement New feature or request label Dec 18, 2023
@azeey
Copy link
Collaborator

azeey commented Dec 18, 2023

One issue with using the mass instead of densities is that it's not clear what to do if you have a link with multiple collisions. How much mass would each collision contribute to the whole?

@scpeters
Copy link
Member Author

One issue with using the mass instead of densities is that it's not clear what to do if you have a link with multiple collisions. How much mass would each collision contribute to the whole?

I tried to express this briefly towards the end of the feature request:

This feature request is proposing that if a user sets //inertial/@auto == 'true' and specifies //inertial/mass, that the density values should be ignored, and the inertial values calculated according to the density that would result in the user-specified mass:

Currently we assume uniform density within each individual shape, but allow each shape to have different densities. I can see two options for allocating a user-specified //inertial/mass between multiple shapes:

  1. Assume uniform density across all shapes. The total volume of all collision shapes is computed and the mass distributed equally between all shapes.
  2. Use the relative values of the density for each collision shape to allocate mass between the shapes. In this case, we would still parse the density values in the same way we do now, but would interpret them in a relative sense, rather than an absolute sense in units of kg/m^3

@azeey
Copy link
Collaborator

azeey commented Dec 18, 2023

Assume uniform density across all shapes. The total volume of all collision shapes is computed and the mass distributed equally between all shapes.

This is possible, but won't be a trivial change because now inertia calculation would have to be a two step process. (1) Ask each collision to compute its volume. For meshes, we need a new API for custom MOI calculators to provide volume. (2) Compute the density and use it to calculate MOI.

Since computing the volume requires loading the mesh and maybe as computationally expensive as calculating the MOI, we should make sure to not duplicate the work.

@scpeters
Copy link
Member Author

scpeters commented Jan 9, 2024

Assume uniform density across all shapes. The total volume of all collision shapes is computed and the mass distributed equally between all shapes.

This is possible, but won't be a trivial change because now inertia calculation would have to be a two step process. (1) Ask each collision to compute its volume. For meshes, we need a new API for custom MOI calculators to provide volume. (2) Compute the density and use it to calculate MOI.

That's a good point. I don't think option 1. that I suggested makes as much sense. I think the following would work though:

Use the relative values of the density for each collision shape to allocate mass between the shapes. In this case, we would still parse the density values in the same way we do now, but would interpret them in a relative sense, rather than an absolute sense in units of kg/m^3

Basically, if we take the auto-computed inertial values based on the existing density parameters and then rescale the auto-computed mass to match the user-specified mass, and then apply that same scaling factor to the moment of inertia values (ixx, iyy, etc.), I think that would work.

@azeey
Copy link
Collaborator

azeey commented Jan 17, 2024

I see what you mean. Yes, I think that would work. One point of clarification: when you say "based on the existing density parameters", would we allow both mass and density to be specified or are you referring to the default values?

@scpeters
Copy link
Member Author

I see what you mean. Yes, I think that would work. One point of clarification: when you say "based on the existing density parameters", would we allow both mass and density to be specified or are you referring to the default values?

yes, I would suggest allowing both mass and density to be specified. If both are specified, the densities would be used for computing the mass distribution (center of mass and moment of inertia matrix), but the densities would not affect the absolute mass value in kg

@SeanCurtis-TRI
Copy link

Two thoughts:

  1. In an ideal world, the volume in consideration is the union of the various collision geometries. Where geometries overlap, it would be unfortunate to count the material in that intersection multiple times.
    • I appreciate that there's a lot of work between easy/practical implementation and this ideal. But it felt worth mentioning.
  2. re: specifying mass and density
    • Given the assumption of homogeneous material, the density won't affect the position of the center of mass. But it could have a significant impact on the apparent mass distribution as encoded in the inertia matrix. If the density and mass are significantly misaligned, you could end up with, for example, a body with small overall mass but ridiculous rotational inertia (easy to translate, hard to rotate), or vice versa, a heavy, almost immovable object that spins when its breathed on.
    • FTR Drake handles this by computing a "unit inertia" and storing the mass independently. Then, the full rotational inertia matrix is the product of the mass with the unit inertia, keeping everything in sync.

@scpeters scpeters mentioned this issue May 23, 2024
8 tasks
@azeey azeey self-assigned this Sep 16, 2024
@azeey
Copy link
Collaborator

azeey commented Sep 16, 2024

@scpeters shall we move this issue to gazebosim/sdformat since it's a feature request?

@scpeters
Copy link
Member Author

Sure, we can move it to sdformat

@azeey azeey transferred this issue from gazebosim/sdf_tutorials Sep 16, 2024
@scpeters
Copy link
Member Author

scpeters commented Oct 7, 2024

  1. In an ideal world, the volume in consideration is the union of the various collision geometries. Where geometries overlap, it would be unfortunate to count the material in that intersection multiple times.

    • I appreciate that there's a lot of work between easy/practical implementation and this ideal. But it felt worth mentioning.

Thanks for bringing this up. You're right that it would be difficult to address. I believe it is also an issue with our current approach to automatic inertial calculation that also computes volumes with density to calculate mass. I think it's out of scope for this issue

@scpeters
Copy link
Member Author

scpeters commented Oct 7, 2024

  1. re: specifying mass and density

    • Given the assumption of homogeneous material, the density won't affect the position of the center of mass. But it could have a significant impact on the apparent mass distribution as encoded in the inertia matrix. If the density and mass are significantly misaligned, you could end up with, for example, a body with small overall mass but ridiculous rotational inertia (easy to translate, hard to rotate), or vice versa, a heavy, almost immovable object that spins when its breathed on.
    • FTR Drake handles this by computing a "unit inertia" and storing the mass independently. Then, the full rotational inertia matrix is the product of the mass with the unit inertia, keeping everything in sync.

thanks for mentioning this as well; I'd like to use the same "unit inertia" approach as Drake to ensure the rotational inertia is properly scaled

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: To do
Development

No branches or pull requests

3 participants