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

Calculate buoyancy for an MHK turbine #957

Merged
merged 107 commits into from
Oct 20, 2022
Merged

Conversation

hkross
Copy link
Contributor

@hkross hkross commented Jan 4, 2022

This pull request is ready to be merged.

Development is complete and all existing regression tests pass. New regression tests have been added, along with baselines. A corresponding pull request (#55) has been added to my r-test repository.

Feature or improvement description

This pull request allows buoyant loads on a marine hydrokinetic turbine to be calculated and adds the following features:

  • Buoyancy parameters for the blade, tower, hub, and nacelle added to the AeroDyn blade and primary input files
  • Buoyancy flag added to the AeroDyn primary input file
  • Buoyant loads on the blade, tower, hub, and nacelle calculated in AeroDyn and added to existing loads
  • Hub and nacelle loads passed to ElastoDyn (glue code changes for nacelle included in prior pull request)
  • Outputs added for buoyant loads
  • Outputs denoted as 'Aero' changed to 'Fld'

Impacted areas of the software

  • AeroDyn module
  • AeroDyn driver
  • OpenFAST glue code

Test results

All existing regression tests have been updated and pass. New regression tests have been added, including input files and baselines.

Check list

Additional supporting information

Update: This bug was fixed by this pull request.
There is a bug in the glue code changes that prevents test cases from converging when buoyancy is turned on. Cases only converge if all degrees of freedom are turned off in ElastoDyn (with the exception of the blades) or if the new code in FAST_Solver.f90 that transfers hub loads from AeroDyn to ElastoDyn is commented out (line 381).

hkross and others added 30 commits December 23, 2020 15:50
…ancy and resolve merge conflict in AeroDyn_IO
@ebranlard ebranlard self-assigned this Oct 17, 2022
@ebranlard
Copy link
Contributor

ebranlard commented Oct 17, 2022

My small comments here would be:

  • I've added some backward compatibility so that blade files without Buoyancy inputs can still be read
  • I would be in favor of having the nacelle NacCenBx,y,z inputs to be on one line instead of 3
  • I would place the Hub and Nacelle inputs above the tower, to have the inputs in that order: Blades > Hub > Nacelle > Tower

I'm eager to get this pull request in, so I can take care of some changes if needed.

@hkross
Copy link
Contributor Author

hkross commented Oct 18, 2022

My small comments here would be:

  • I've added some backward compatibility so that blade files without Buoyancy inputs can still be read
  • I would be in favor of having the nacelle NacCenBx,y,z inputs to be on one line instead of 3
  • I would place the Hub and Nacelle inputs above the tower, to have the inputs in that order: Blades > Hub > Nacelle > Tower

I'm eager to get this pull request in, so I can take care of some changes if needed.

Thanks, @ebranlard. I can take of the input file changes for this if you can make the code modifications. Would that work?

@ebranlard
Copy link
Contributor

No worries, I'm happy to do all of it.
Do you think the changes would make sense? I'm waiting for @jjonkman feedback as well.

@rafmudaf rafmudaf modified the milestones: v3.3.0, v3.4.0 Oct 18, 2022
@hkross
Copy link
Contributor Author

hkross commented Oct 19, 2022

I am fine with either option for both the NacCenBx,y,z inputs and the order of inputs for the tower, hub, and nacelle. The main motivation for putting the hub/nacelle inputs after the tower was that hub/nacelle inputs are only relevant for MHK turbines, whereas the tower inputs are more generally used. So, it made sense to me to have them after the tower since they will more often be neglected. But, I am fine with having it either way.

@ebranlard
Copy link
Contributor

I guessed so, in the future we can add aerodynamics loads on the nacelle, that's why I think it makes sense to have it higher up.

@ebranlard
Copy link
Contributor

I will take care of the changes

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.

I approve this PR. I heard from @hkross that her recent changes to the buoyancy calculation solved the problem with residual moments that was delaying the merge of this pull request.

See two minor comments on the documentation below, which I assume can be easily addressed before merging.

I agree with moving the hub and nacelle inputs up, between the blade and tower inputs before merging. This will make more sense once hub and nacelle aerodynamics are added to AeroDyn in the future.

I don't have a strong preference whether NacCenBx,y,z is one line or three in the input file.

Looking forward to getting this new functionality merged in!

docs/source/user/aerodyn/introduction.rst Outdated Show resolved Hide resolved
docs/source/user/aerodyn/modeling.rst Outdated Show resolved Hide resolved
@hkross
Copy link
Contributor Author

hkross commented Oct 19, 2022

I will take care of the changes

Thanks, @ebranlard. I can take care of the documentation changes @jjonkman suggested.

@hkross
Copy link
Contributor Author

hkross commented Oct 20, 2022

@ebranlard I changed a couple small things in the documentation. Otherwise, all the changes look good.

@ebranlard ebranlard merged commit ce24848 into OpenFAST:dev Oct 20, 2022
@hkross hkross deleted the feature/Buoyancy branch October 25, 2022 00:21
@rafmudaf rafmudaf mentioned this pull request Oct 27, 2022
10 tasks
@andrew-platt andrew-platt mentioned this pull request Dec 19, 2022
10 tasks
@andrew-platt andrew-platt mentioned this pull request Feb 10, 2023
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants