-
Notifications
You must be signed in to change notification settings - Fork 184
Fix the transpose of linear restoring matrix to make roll mode rows to be 0 #1032
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
Fix the transpose of linear restoring matrix to make roll mode rows to be 0 #1032
Conversation
source/functions/BEMIO/readBEMIOH5.m
Outdated
| % Read hydrostatic stiffness | ||
| hydroData.hydro_coeffs.linear_restoring_stiffness = reverseDimensionOrder(h5read(filename, [h5BodyName '/hydro_coeffs/linear_restoring_stiffness'])); | ||
|
|
||
| hydroData.hydro_coeffs.linear_restoring_stiffness(6:6:end,:) = 0*hydroData.hydro_coeffs.linear_restoring_stiffness(6:6:end,:); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@salhus Is there a reason you are using the (6:6:end,:) rather than (6,:) as you are just trying to zero the last row of the 6x6 hydrostatic stiffness matrix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @nathanmtom
I left that as a safe-guard thinking if the matrix was
I just made a commit that only does that to the
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@salhus Thanks for the clarification, I see why you chose to implement the prior option; however, I do believe that when this function is called it requires the body number and should only have a 6x6 hydrostatic matrix upon import. If you can double check this, I think sticking with the (6,:) is easier to read, but if we are indeed getting multiple bodies read in then we should stick with your implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nathanmtom
Thanks for pointing this out. The imports do happen on a body by body basis, so the
|
@salhus Thanks for looking into the issue and submitting a fix. I've verified that the additional line of code you have included will zero out the last row of the 6x6 hydrostatic restoring matrix. Which is consistent that the yaw-roll and yaw-pitch coupling terms in the 6,4 and 6,5 indices are zero based on the WAMIT documentation you have cited. Please review my one comment above on how you are handling the matrix indices as my recommendation seems more direct, but I might not understand the reason behind that implementation. After we resolve that minor comment this would be fine to merge. |
only change the last row, since the restoring matrix is always 6x6
|
Thanks @salhus for confirming that the function imports body hydrostatics one at a time. I am happy with the implementation and will merge this PR. |
|
@nathanmtom Thanks Nathan. |
The roll mode rows should be 0.$X(1:nFreq, b, a)$ becomes $X(a, b, 1:nFreq)$ . This works well due to matrix symmetry. However, the roll row for linear restoration matrix must be 0.
The readBEMIOh5 function uses the permute command to make matrix dimensions compatible to WEC-Sim. This essentially flips the matrix, such that,
This follows from Issue #1028