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

[BugFix] incorrect pitch/roll moments on tapered elements crossing water line #687

Merged
merged 2 commits into from
Mar 19, 2021

Conversation

andrew-platt
Copy link
Collaborator

@andrew-platt andrew-platt commented Mar 19, 2021

** This pull request is ready to be merged**

Feature or improvement description

There were two negative signs missing in the calculation of the centroid for tapered members crossing the still water line, and one incorrect variable name. The negative signs were missing in the original implementation plan.

Related issue, if one exists
This had been reported by a collaborator on an ARPA-E ATLANTIS project.

Impacted areas of the software
HydroDyn module. Only tapered members crossing the still water surface are affected

Additional supporting information

Test results, if applicable
Added a very simple test case of a tapered cylinder (0.1m taper over 40 m) with a prescribed pitch motion.

Original results showing pitching moment that was way too large:
hd_taper_HydroMyi--orig

Corrected results in this PR:
hd_taper_HydroMyi--corrected

There were two negative signs missing in the code (this had been missing in the documentation originally for this equation)
@andrew-platt
Copy link
Collaborator Author

@mattEhall, take a look at the code change for updating the plan. This now matches your python script (I can upload the pages of algebra if you want the proof).

Should we add some of that derivation into the docs at some point?

Copy link
Contributor

@HaymanConsulting HaymanConsulting left a comment

Choose a reason for hiding this comment

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

Looks like in addition to the negative signs (which, as you state, did not appear in the implementation plan), there was an additional typo in the code, where the value of r1 was used instead of rh. The implementation plan does, indeed, call out rh in that particular term.

I'll leave the verification of the final form of the equation to Matt H.

@andrew-platt andrew-platt marked this pull request as ready for review March 19, 2021 15:32
@mattEhall
Copy link
Contributor

These corrections look good, Andy. They match what I have in a Python verification script and match the corrected derivation. The trends in the plot of corrected moments look right. Thanks for tracking the errors down!

Copy link
Collaborator

@jjonkman jjonkman 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 tracking down this bug! This is one part of the implementation plan that I was not able to check in detail.

@andrew-platt andrew-platt merged commit 31b6101 into OpenFAST:dev Mar 19, 2021
@andrew-platt andrew-platt deleted the b/HD_taper branch March 19, 2021 20:44
@rafmudaf rafmudaf mentioned this pull request May 12, 2021
11 tasks
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.

4 participants