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
New AeroDyn input file exposing new BEM options (polar BEM, skew momentum correction, sector averaging) #1909
base: dev-unstable-pointers
Are you sure you want to change the base?
Conversation
9ee85b2
to
e83e8a4
Compare
This pul request is ready for review. Becasue it relies on dev-unstable-pointers, I would like to recommend merging it now at this point, so that others start to use it and potentially give feedback / bug fixes. In a future pull request, I can work on a script to convert the AeroDyn input files and do that for the r-test inputs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, this reformatted AeroDyn input file looks great to me! Please find a few minor comments below.
====== Blade-Element/Momentum Theory Options ====================================================== [unused when WakeMod=0 or 3, except for BEM_Mod] | ||
2 BEM_Mod - BEM model {1=legacy NoSweepPitchTwist, 2=polar} (switch) [used for all Wake_Mod to determine output coordinate system] | ||
--- Skew correction | ||
1 Skew_Mod - Skew model {0=No skew model, -1=Remove non-normal component for linearization, 1=skew model active} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the difference between Skew_Mod = 0 and Skew_Mod = 1 with SkewModCorr = False and SkewRedistr_Mod = 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, they would be the same. Skew_Mod is like a master switch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. I suggest adding a sentence in the input file documentation to clarify this point.
4 tau1_const - Time constant for DBEMT (s) [used only when WakeMod=2 and DBEMT_Mod=1 or 3] | ||
--- Shear correction | ||
False SectAvg - Use sector averaging (flag) | ||
1 SectAvgWeighting - Weighting function for sector average {1=Uniform} within a sector centered on the blade (switch) [used only when SectAvg=True] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were other SectAvgWeighting options implemented? Wasn't the angle range of the sector averaging implemented? Or do we want to keep this simple for now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently I have two hidden inputs:
SA_PsiBwd
and SA_PsiFwd
We disagreed on their use (you advocated for 360/nB) and wanted to rely on the weighting method only.
I think the extent is quite useful, and saves in computational time (if we want to focus close to the blade).
I would be in favor of introducing these inputs below. The alternative is to keep them hidden for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I generally prefer not having "hidden" inputs. I would be in favor of exposing the inputs you recommend (SA_PsiBwd
, SA_PsiFwd
) directly within the input file.
ToDo items
|
I was feeling lazy and didn't want to fuss with appropriately appending T/F or numbers for each value getting written out, so I just used an internal write to a string to make this simple.
Also changed a couple of `print*` to `call WrScr()`
@ebranlard, if/when you are done with the minor updates, I will plan on merging this in the next day or two. |
This pull request is ready to be merged
See :
#1895
Feature or improvement description
Related issue, if one exists
Impacted areas of the software
AeroDyn
Test results
No change expected as this introduces new options, and the input file is read as backward compatible.
Future pull request