Skip to content

Conversation

@akeeste
Copy link
Contributor

@akeeste akeeste commented Feb 25, 2025

This PR fixes a minor bug in the treatment of the inertia products in body.adjustMassMatrix(). See https://wec-sim.github.io/WEC-Sim/main/developer/advanced_features.html#theoretical-implementation for the documented reasoning which this update aims to match.

@akeeste akeeste added Bug bug in WEC-Sim source, high priority Added Mass related to added mass Body Class Body Class (bodyClass.m) labels Feb 25, 2025
@akeeste
Copy link
Contributor Author

akeeste commented Feb 25, 2025

Changing back to draft while I investigate the implementation of added mass adjustments with body interactions

@akeeste akeeste marked this pull request as draft February 25, 2025 17:22
@akeeste
Copy link
Contributor Author

akeeste commented Feb 25, 2025

Ready for review again. I confirmed the implementation of our added mass adjustments in b2b cases and added to our documentation explaining it.

@akeeste akeeste marked this pull request as ready for review February 25, 2025 19:30
@kmruehl kmruehl self-assigned this Mar 10, 2025
@kmruehl kmruehl self-requested a review March 10, 2025 18:57
@kmruehl
Copy link
Collaborator

kmruehl commented Mar 10, 2025

@akeeste after reviewing the proposed documentation revisions, the bodyClass revisions in this PR appears to be good, but it would be helpful if you could verbally explain the issue and proposed solution within the PR for future reference/clarity. Thank you!

@akeeste
Copy link
Contributor Author

akeeste commented Mar 11, 2025

Correcting my above comment:
I was referencing our docs originally, which were incorrect. The bodyClass implementation and treatment of inertia products in the adjustMassMatrix function was correct, but our developer manual section on WEC-Sim's added mass treatment was incorrectly writing the inertia tensor as if it was a skew-symmetric matrix. It is not. The inertia tensor is a symmetric matrix. Our source was right, the docs were wrong.

Those docs are now corrected and the inertia tensor is written correctly

@akeeste akeeste added Documentation related to docs Added Mass related to added mass and removed Bug bug in WEC-Sim source, high priority Added Mass related to added mass Body Class Body Class (bodyClass.m) labels Mar 11, 2025
@akeeste
Copy link
Contributor Author

akeeste commented Mar 11, 2025

@kmruehl FYI, see above comment for explanation. The docs build fine so this should be good to merge

@kmruehl
Copy link
Collaborator

kmruehl commented Mar 11, 2025

@akeeste thank you for the clarification and for reverting the revisions to the bodyClass. Once the tests pass, I will merge.

@kmruehl kmruehl merged commit d8a2314 into WEC-Sim:main Mar 11, 2025
10 checks passed
@akeeste akeeste deleted the fix_adjustMassMatrix branch March 11, 2025 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Added Mass related to added mass Documentation related to docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants