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 hydrodynamic added mass #76

Merged
merged 14 commits into from Dec 12, 2022
Merged

Conversation

chapulina
Copy link
Contributor

@chapulina chapulina commented May 31, 2022

I'm looking for early feedback on the proposal. I'm planning to circulate this with the ROS Maritime Working Group for feedback after the initial review.

Rendered preview:

http://sdformat.org/tutorials?tut=added_mass_proposal&branch=chapulina/added_mass

Signed-off-by: Louise Poubel <louise@openrobotics.org>
@osrf-triage osrf-triage added this to Inbox in Core development May 31, 2022
@chapulina chapulina self-assigned this May 31, 2022
@chapulina chapulina moved this from Inbox to In review in Core development May 31, 2022
@scpeters scpeters changed the title Proposal for added mass Proposal for hydrodynamic added mass Jun 1, 2022
@scpeters
Copy link
Member

scpeters commented Jun 1, 2022

For people unfamiliar with hydrodynamics, the term "added mass" is not very clear. Can you use the phrase "hydrodynamic added mass" in titles and when introducing the concept to help with clarifying this?

@hamilton8415
Copy link
Collaborator

Thanks Steve, I will work through this and add some clarifying details as you suggest. Trick will be for me to keep it short, suggestions on how general an audience to target are welcome.

@scpeters
Copy link
Member

scpeters commented Jun 1, 2022

Thanks Steve, I will work through this and add some clarifying details as you suggest. Trick will be for me to keep it short, suggestions on how general an audience to target are welcome.

adding the word "hydrodynamic" is probably enough for me at first glance, but I'll add additional questions as they occur to me

…red things some and used the usual greek letter mu in place of n, changed in the description and the xml example.

This is fairly standard nomenclature so I think <mu11>1.0<\mu11> in the xml should be clear.  This commit does not address changes in the C++ API, I found the comments there a bit confusing
as they refer to the center of mass coordinate system of the body as "an inertia frame", which it usually won't be in the usual meaning of that term.
Perhaps I am misunderstanding something, but am intending to find out if this is a comment adapted from previous code and I'm mis-understanding something, or if it's a new comment.
added_mass/proposal.md Outdated Show resolved Hide resolved
added_mass/proposal.md Outdated Show resolved Hide resolved
added_mass/proposal.md Outdated Show resolved Hide resolved
added_mass/proposal.md Outdated Show resolved Hide resolved
added_mass/proposal.md Outdated Show resolved Hide resolved
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina
Copy link
Contributor Author

I pushed some more paragraphs in 5c36c87 explaining the motivation for storing the added mass terms separately from the remaining inertial terms, even though they will be combined when fed to the physics engine.

I also updated the target version to Garden, since any changes to math::Inertial will break ABI. This means we have 1 month to get at least the math changes in.

While many physics engines may not have direct support for added mass, they often
support directly setting each element of the resulting \\(\bf{M} + \bf{\mu}\\)
matrix that's multiplied by acceleration. That's the case for
[DART](https://dartsim.github.io/dart/main/d6/d91/classdart_1_1dynamics_1_1Inertia.html#aa4661f9950d5958efa4c8609d42aedf7),
Copy link
Member

Choose a reason for hiding this comment

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

I was just looking more closely at DART's inertia code, and it looks like it will print warnings if a spatial inertia matrix has nonzero values in off-diagonal parts of the translational 3x3 block-diagonal matrix (xy, xz, yz) or the diagonal parts of the block-off-diagonal 3x3 matrix (xp, yq, zr). For rigid bodies, these terms are all 0, so I think DART will complain a bit about using them for fluid added mass, though it might still work despite the complaints, depending on what assumptions they make when inverting the spatial inertia matrix

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that's a concern that @hamilton8415 and I have had from the beginning. Hopefully it's just a matter of suppressing those warnings. It would be nice to have a prototype implementation out soon so we can have an idea of what breaks and what doesn't.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From my initial testing, all we need to do is suppress those warnings, see

I'll check with @hamilton8415 in the coming days if the resulting behavior is what's expected though.

Signed-off-by: Louise Poubel <louise@openrobotics.org>
added_mass/proposal.md Outdated Show resolved Hide resolved
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
added_mass/proposal.md Outdated Show resolved Hide resolved
Copy link

@arjo129 arjo129 left a comment

Choose a reason for hiding this comment

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

Is it worth describing the units here.

Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina
Copy link
Contributor Author

While working on the implementation I realized that since the added mass coefficients are fluid-dependent, it would be convenient to expose the fluid density as a separate parameter. This way, it's trivial to adapt an SDF file for different fluids.

I made the changes for that in 60866d4, and will start working on the implementation. I'd appreciate some reviews, since I haven't seen the added mass matrix broken apart like this anywhere in the literature. I may be doing something that may not hold for every use case, or that will make it difficult to export values from other software directly into an SDF file.

CC @andermi

Signed-off-by: Louise Poubel <louise@openrobotics.org>
@andermi
Copy link

andermi commented Jul 18, 2022

Yes, some bodies will have empirical added mass matrices from CFD or pool tests, and some easier shapes will have closed-form dependent on fluid density. Perhaps, we could support an explicit matrix and a formulaic one?

Copy link
Member

@scpeters scpeters left a comment

Choose a reason for hiding this comment

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

regarding the addition of the density parameter; if it is optional and defaults to a value of 1.0, then users can simply omit it if they prefer to specify the fluid-dependent values directly

added_mass/proposal.md Outdated Show resolved Hide resolved
added_mass/proposal.md Outdated Show resolved Hide resolved
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina
Copy link
Contributor Author

expose the fluid density as a separate parameter

After talking to @andermi today we figured that wasn't a very good idea, so I removed the separate parameter.

@chapulina chapulina added the enhancement New feature or request label Jul 23, 2022
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@mjcarroll
Copy link

After landing the implementation PR, we are going to give this one more review to confirm that the implementation and documentation match.


Newton's second law describes how forces affect a body's motion in an inertial frame. It can be written as:

$$ (\bf{M} + \bf{\mu}) \ddot{\bf x} = \sum{F({\bf x}, t)} $$

Choose a reason for hiding this comment

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

Note that this equation is only valid for translation; for the rotation of a rigid body, this becomes Euler's equations; and in the presence of cross-terms in the spatial inertia matrix, this becomes equation (2.90) from [1].

@mjcarroll mjcarroll merged commit 90dac07 into master Dec 12, 2022
Core development automation moved this from In review to Done Dec 12, 2022
@mjcarroll mjcarroll deleted the chapulina/added_mass branch December 12, 2022 21:58
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
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

7 participants