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

MoorDyn updates and bug fixes #1727

Merged
merged 15 commits into from
Oct 2, 2023
Merged

Conversation

RyanDavies19
Copy link
Contributor

@RyanDavies19 RyanDavies19 commented Aug 9, 2023

Feature or improvement description
Finished Connections to Points:
The transition to using the word points from using the word connections has been completed. Going forward all development should use the word points.

Standardized catenary solver:
The catenary solver has been standardized between MoorDynC and MoorDynF to produce the same results.

Standardized outputs:
MoorDynF and MoorDynC output flags have been standardized according to the following table:

Fixed input units:
MoordynF and MoorDynC now correctly take degrees as the input unit for body orientations (roll pitch yaw). Previously the input file listed degrees but internally the values were interpreted as radians (FloatingArrayDesign/MoorDyn#108).

Bug Fixes:
Resolves a body added mass bug, a rod Aq bug, rod inertia bug, and typos in comments:

  1. The body added mass calculation was adding a volume to the mass, which is dimensionally incorrect, and was missing the water density and the Ca coefficient.
  2. The rod Aq bug was identified by Lu Wang at NREL. The force labeled Froude-Krylov force is actually the long-wave diffraction force. The force labeled dynamic pressure is actually the Froude Krylov force. The 1 that was added to CaEnd was a mistake because it represents the FK contribution which had already been accounted for in the dynamic pressure calculation (now FK force). See Eq. (32) on page 51 of the HydroDyn manual for the detailed equations: https://www.nrel.gov/wind/nwtc/assets/downloads/HydroDyn/HydroDyn_Manual.pdf
  3. The rod inertia bug has been discussed in PR 93 on the C++ repo (Rod dynamics and outputs updates FloatingArrayDesign/MoorDyn#93). The lumped mass approach incorrectly handles the end nodes moment of inertias and incorrectly applies the parallel axis theorem for the rod. External contributors to the C++ code have introduced a correction term that @mattEhall and I have verified produces the correct moment of inertia for the rod.
  4. The axial drag value was missing a 0.5 coefficient as discussed in this thread. It has been added accordingly.

Related issue, if one exists
n/a

Impacted areas of the software
MoorDyn

Additional supporting information
The new output flags have been added to the MoorDyn documentation.

Test results, if applicable
n/a

@andrew-platt
Copy link
Collaborator

@mattEhall, can you review this?

@andrew-platt
Copy link
Collaborator

@RyanDavies19, are there any input file changes required for MD? It also looks like this may be changing the regression test results for MoorDyn (not surprising given the bugfixes).

@RyanDavies19
Copy link
Contributor Author

@andrew-platt No input file format changes for MD, other than the possible flags in the output options section are slightly different and have more options. This is being updated in the MD C++ documentation that the MD OpenFAST documentation page is linked to.

@andrew-platt
Copy link
Collaborator

There are some modifications needed in FAST.Farm as well: https://github.com/OpenFAST/openfast/actions/runs/5812836020/job/15761098777?pr=1727

@RyanDavies19
Copy link
Contributor Author

Fixed that FAST.farm error. It needed to be updated to match the points convention.

@andrew-platt
Copy link
Collaborator

Thanks @RyanDavies19! I'll take a look at the differences in the regression test results and verify they make sense. Then probably do a PR to your branch with the regression test baseline update.

Since this is a bug fix, I'm going to put it into the rc-3.5.1 branch for our next release.

@andrew-platt andrew-platt changed the base branch from main to rc-3.5.1 August 10, 2023 02:03
@mattEhall
Copy link
Contributor

Hi Andy, I've reviewed and approve this once all tests are passing.
Thanks Ryan for doing all this, and Andy for catching the FAST.Farm adjustment.

@RyanDavies19
Copy link
Contributor Author

@andrew-platt FYI I just added a new fix. MoorDyn was taking body orientation inputs as radians when it should have been degrees. Chatted with Matt and Lu about this.

@luwang00
Copy link
Contributor

Hi @RyanDavies19, I just want to check with you whether the expression for the axial drag force on rod ends is as intended on the following two lines:
https://github.com/RyanDavies19/MoorDynF_ryan/blob/357d82717e41af504f044e33bb0d0d0bca2151d9/modules/moordyn/src/MoorDyn_Rod.f90#L830
https://github.com/RyanDavies19/MoorDynF_ryan/blob/357d82717e41af504f044e33bb0d0d0bca2151d9/modules/moordyn/src/MoorDyn_Rod.f90#L860
It looks like the drag force is missing a coefficient of 0.5 in front (or 0.25 if you follow the HydroDyn convention).

@mattEhall
Copy link
Contributor

@luwang00, you're right, a factor of 0.5 is missing!
(I think this single-ended convention is a good fit for MoorDyn, rather than the HydroDyn convention of using 0.25.)

@RyanDavies19, if this sounds right to you too would you mind making this fix and also checking MD-C?
Also, I noticed a couple ancient commits sitting on my OpenFAST dev branch. This PR is probably a good place to include them. Could you please cherry-pick in mattEhall@70c011a, and let's chat about a few updates to the warnings and messages.

@RyanDavies19
Copy link
Contributor Author

RyanDavies19 commented Sep 18, 2023

Hi @RyanDavies19, I just want to check with you whether the expression for the axial drag force on rod ends is as intended on the following two lines:
https://github.com/RyanDavies19/MoorDynF_ryan/blob/357d82717e41af504f044e33bb0d0d0bca2151d9/modules/moordyn/src/MoorDyn_Rod.f90#L830
https://github.com/RyanDavies19/MoorDynF_ryan/blob/357d82717e41af504f044e33bb0d0d0bca2151d9/modules/moordyn/src/MoorDyn_Rod.f90#L860
It looks like the drag force is missing a coefficient of 0.5 in front (or 0.25 if you follow the HydroDyn convention).

@luwang00 I didn't know those were off, good catch. Is the correct fix then to use HydroDyn coefficients as shown below?

Line 830:

! axial drag
            Rod%Dq(:,I) = Rod%Dq(:,I) + 0.25 * VOF * 0.25* Pi*Rod%d*Rod%d * p%rhoW*Rod%CdEnd * MagVq * Vq

@RyanDavies19
Copy link
Contributor Author

RyanDavies19 commented Sep 18, 2023

@mattEhall will do and add to this branch. Is it just the one commit you linked? Happy to chat about changes to error handling, most everything was with regards to how things were printed.

@luwang00
Copy link
Contributor

Hi @RyanDavies19,

I think @mattEhall would prefer using a coefficient of 0.5 instead of 0.25, i.e.,
Rod%Dq(:,I) = Rod%Dq(:,I) + 0.5 * VOF * 0.25* Pi*Rod%d*Rod%d * p%rhoW*Rod%CdEnd * MagVq * Vq
@mattEhall, can you confirm? Thanks.

@RyanDavies19
Copy link
Contributor Author

@luwang00 I chatted with Matt, 0.5 is what he said is best.

@luwang00
Copy link
Contributor

@RyanDavies19 Thanks for confirming. Looks good to me.

@RyanDavies19
Copy link
Contributor Author

This is ready to merge. The fix to the catenary solver was changing the units of the line wet weight passed into the catenary from kg/m to N/m (Used by MoorPy and MoorDynC v1/v2). Prior to this, MoorDynF was not initializing the line tension correctly as seen in the figure below. Discussed this issue with Lu Wang and Stein Housner and they are in agreement.

Screenshot 2023-09-25 at 3 50 05 PM

With this change, the initial tensions look like the following:

Figure_2

The difference is due to a bug in the v2 C code, as MoorDynF now matches MoorDynC v1.

@andrew-platt andrew-platt marked this pull request as ready for review September 29, 2023 18:32
Copy link
Collaborator

@andrew-platt andrew-platt left a comment

Choose a reason for hiding this comment

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

There are a few regression tests that need updating. I'll post a PR to this branch for them.

andrew-platt added a commit to OpenFAST/r-test that referenced this pull request Oct 2, 2023
@andrew-platt andrew-platt merged commit 7d09eb9 into OpenFAST:rc-3.5.1 Oct 2, 2023
19 checks passed
@andrew-platt andrew-platt mentioned this pull request Oct 19, 2023
19 tasks
@RBergua
Copy link
Contributor

RBergua commented Nov 2, 2023

According to the documentation for MoorDyn v2 (https://moordyn.readthedocs.io/en/latest/inputs.html#points-list), it seems that the attachment could be defined as “Fixed”, “Vessel”, “Connect”.
image

However, OpenFAST v3.5.1 does not accept "Connect".
image

It seems that this "Connect" has been removed and "Coupled" should be used instead. Right?

@RyanDavies19
Copy link
Contributor Author

@RBergua It depends what you are looking for. Previous versions of MoorDyn used the word 'connect', which we have now transitioned to 'point'. Connect referred to a free point, and the key words 'free', 'point', or 'p' will give the same behavior in MoorDyn as the old 'connect' keyword would. In changing to points I tried to keep things backwards compatible, but missed this in the fortran code (connect is still and option in the C code).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants