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

Proposal for Automatic Calculation of Moments of Inertia for SDFormat Links #92

Merged
merged 23 commits into from
Sep 14, 2023

Conversation

jasmeet0915
Copy link
Contributor

@jasmeet0915 jasmeet0915 commented Jun 13, 2023

This proposal is in regard to GSoC'23 Project titled: "Automatically Computing Moments of Inerial for SDFormat Links"

Summary

The proposal suggests adding new attributes and elements to support the automatic calculation of Moments of Inertia and Products of Inertia for a link in SDFormat. The proposal targets SDFormat 1.11 and libsdformat14.

Preview

http://sdformat.org/tutorials?tut=auto_inertial_params_proposal&cat=pose_semantics_docs&branch=jasmeet%2Fauto_inertial_params&repo=https%3A%2F%2Fgithub.com%2Fjasmeet0915%2Fsdf_tutorials

Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
@osrf-triage osrf-triage added this to Inbox in Core development Jun 13, 2023
Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
@azeey azeey moved this from Inbox to In review in Core development Jun 26, 2023
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved

1. An `auto` parameter for the `<inertia>` tag that would tell `libsdformat` to calculate Inertia matrix values automatically for the respective link.

2. A `//link/collision/material_density` tag for specifying the density of the collision geometry. This density value would be used to calculate the inertial parameters of the respective collision geometry. Adding this as part of the `<collision>` tag would allow a user to simulate links with different material types for different collisions. By default, the value of density would be set equal to that of water which is 1000 kg/m^3.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A link may contain 0 or multiple collision tags. The proposal should specify how inertial properties are computed in cases when there is not exactly 1 collision. If there are no collisions and the auto attribute is set to true, we could return an error. If there are multiple collisions, we could aggregate them using the gz::math::Inertial class and its + operator (see examples from its unit test)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@scpeters Thank you for the feedback. The case to handle multiple collisions is already under development here and would be added in this PR. I'll also update the PR to handle a case of 0 collisions by returning an error and would update the proposal accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the section to include more information about different scenarios (multiple or no collisions) in 8b902d8

auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
@azeey azeey added the beta label Jul 31, 2023
Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
…f <inertia> element

Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
quarkytale and others added 4 commits August 24, 2023 14:35
Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com>
Co-authored-by: Steve Peters <computersthatmove@gmail.com>
Signed-off-by: Dharini Dutia <dharinidutia@gmail.com>
Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
…Inertial() section

Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
jasmeet0915 and others added 3 commits August 28, 2023 19:30
Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
Comment on lines 395 to 396
Since this method uses the vertex data for calculations, a high vertex count would be
required for near-ideal values. For eg, in case of a cylinder, it was observed that with
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is important for usage right? Should this be documented some place else as well? Probably a Limitations or Considerations sub-section in implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Proposed Implementation section of the proposal focuses more on the implementation from libsdformat's side. Therefore, instead of adding it there, I created a sub-section in the Integration-based Numerical Method and added this point. I have added some other points as well that explain this method's limitations and suitable use cases. The changes can be found in fca06ae

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great thanks!

auto_inertial_params/proposal.md Show resolved Hide resolved
…ion method

Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
Copy link
Contributor

@quarkytale quarkytale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for iterating! Looks good to me 💯

Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
Copy link
Collaborator

@azeey azeey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great overall. Just a couple of minor points. I would also ask you rename SDF to SDFormat throughout the document.

auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
auto_inertial_params/proposal.md Show resolved Hide resolved
auto_inertial_params/proposal.md Outdated Show resolved Hide resolved
Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
…tion, updated configuration docs

Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
@jasmeet0915
Copy link
Contributor Author

Looks great overall. Just a couple of minor points. I would also ask you rename SDF to SDFormat throughout the document.

Replaced all SDF with SDFormat in 52b582e

Copy link
Collaborator

@azeey azeey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thanks for iterating patiently.

@azeey azeey merged commit 2ba0b3a into gazebosim:master Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

5 participants