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 fix: BEMT was disabled for negative inflow #1037

Merged
merged 5 commits into from
Sep 21, 2022

Conversation

bjonkman
Copy link
Contributor

@bjonkman bjonkman commented Mar 9, 2022

Feature or improvement description
The check to turn off BEM for low TSRs should have been using ABS(TSR) instead of TSR so that BEM is not disabled for all negative TSR values.

Related issue, if one exists
#940

Impacted areas of the software
AeroDyn

Additional supporting information

Test results, if applicable
Tests where TSR < -1.5 will have differences due to BEM now turning back on at that point.

AD Timeseries Shutdown case (AD driver):
bokeh_plot(4)
bokeh_plot(3)

ad_timeseries_shutdown

AD BAR RNA motion case (AD driver):
bokeh_plot(2)
bokeh_plot(1)
ad_BAR_RNAmotion

The check to turn off BEM for low TSRs should have been using `ABS(TSR)` instead of `TSR` so that BEM is not disabled for all negative TSR values.

OpenFAST#940
@andrew-platt andrew-platt self-assigned this Mar 9, 2022
@andrew-platt andrew-platt added Module: AeroDyn Approved: Update baselines Pull request is ready for updated regression test baselines labels Mar 9, 2022
@andrew-platt
Copy link
Collaborator

Merge this around the same time we do PR #932

@rafmudaf rafmudaf modified the milestones: v3.4.0, v3.3.0 Sep 20, 2022
@rafmudaf
Copy link
Collaborator

@bjonkman could you update the r-test commit here to 1db296f2? I can push to your branch from the GitHub web editor but not from my command line, so I can't make the change to r-test directly.

@rafmudaf
Copy link
Collaborator

Actually, it looks like you also updated the r-test commit previously. I've updated the AeroDyn cases that are currently failing in 1db296f2. If I'm understanding the state of this pull request correctly, you could cherry pick that commit to your r-test branch.

@bjonkman
Copy link
Contributor Author

bjonkman commented Sep 20, 2022

Actually, it looks like you also updated the r-test commit previously.

That "update" was just to get the submodule to point to the most recent dev commit, which it should have done when I merged the branch (on the next commit). I just did an extra commit because the submodule initialization was being a pain on my computer today.

Anyway, it should be pointing to your updated r-test commit now. Thanks for updating it!

@rafmudaf rafmudaf merged commit e3f401f into OpenFAST:dev Sep 21, 2022
@bjonkman bjonkman deleted the b/BEM_TSR_turnoff branch September 26, 2022 20:44
@rafmudaf rafmudaf mentioned this pull request Oct 27, 2022
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved: Update baselines Pull request is ready for updated regression test baselines Module: AeroDyn
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants