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

Bug: AeroDyn-Inflow WriteOutput Value Ordering #2457

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

deslaughter
Copy link
Contributor

This PR is ready to merge.

Feature or improvement description

This PR fixes a bug in the AeroDyn-Inflow library (AeroDyn_Inflow.f90) where the order of WriteOutput did not match WriteOutputHdr when InflowWind outputs were requested. InflowWind outputs were put at the end of WriteOutput, but the header indicated that InflowWind values should appear before AeroDyn values.

Impacted areas of the software

AeroDyn_Inflow.f90

Additional supporting information

This issue only occurs when using AeroDyn-Inflow directly or AeroDyn-Inflow C Bindings when outputs have been requested from InflowWind and AeroDyn. The AeroDyn Driver output is not affected as that program assembles its output array directly.

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.

Looks good.

One modification you could do if you want.

modules/aerodyn/src/AeroDyn_Inflow.f90 Outdated Show resolved Hide resolved
@bjonkman
Copy link
Contributor

bjonkman commented Oct 9, 2024

This is interesting. The AeroDyn driver uses the headers that ADI assembles. I wonder why the AD driver assembles this array a second time instead of using this one from ADI. It was an issue in #2287.

@andrew-platt
Copy link
Collaborator

I knew this issue sounded familiar yesterday when were diagnosing it for ADI c-bindings -- I couldn't find the issue number at the time.

The evolution of the driver and ADI library was convoluted with both @ebranlard and myself working on them. During the writing of the c-bindings library, I had duplicated some code that was in the driver. At that point we decided to collect some of the common pieces between the c-bindings and driver into the AeroDyn_Inflow.f90 library. However neither of us fully completed revisions in the driver or c-bindings (it ended up at the bottom of the list of code cleanup priorities to do when we have nothing else to do), so I'm not surprised that the driver is handling this differently than c-bindings.

…pointer as the py_ad_* regression test references needed to change
@andrew-platt andrew-platt merged commit 7fc2930 into OpenFAST:dev Oct 10, 2024
21 checks passed
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.

3 participants