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

MHK: add parameters for readability #1629

Merged
merged 2 commits into from
Jun 16, 2023
Merged

Conversation

bjonkman
Copy link
Contributor

Feature or improvement description
This PR is for code readability.

  • It replaces the MHK=={0|1|2} statements using parameters for the 3 allowed values of MHK. I am not thrilled with putting the MHK parameters in NWTC Library, but they are used in many different modules so that's where I put them (at least for now).
  • It also simplifies some of the logic statements. For example IF (A) THEN ... ELSEIF (NOT A) statements are now IF (A) THEN... ELSE, avoiding a second check that must always be true.

Related issue, if one exists
None.

Impacted areas of the software
This impacts the places that check the value of the MHK variable: ElastoDyn, AeroDyn, InflowWind, and OpenFAST glue code

Additional supporting information

Test results, if applicable
This shouldn't affect any tests. Tests have have failed are related to GH Actions getting the wrong patch version of python.

- I don't really like these parameters in NWTC Library, but they seem to be used in many different modules, so that's the easiest place for now
- some of the IF statements had redundant ELSEIF  instead of just ELSE, so I simplified them from (IF A ... ELSEIF NOT A...) to (IF A ... ELSE...)
@andrew-platt andrew-platt self-requested a review June 15, 2023 15:39
@andrew-platt andrew-platt self-assigned this Jun 15, 2023
@andrew-platt andrew-platt added this to the v4.0.0 milestone Jun 15, 2023
@andrew-platt
Copy link
Collaborator

@hkross, do you want to take a quick look?

@hkross
Copy link
Contributor

hkross commented Jun 15, 2023

Thanks for doing this, @bjonkman! The MHK flag is used in a couple of places in AeroDyn_Driver_Subs as well. Would you prefer to change these to parameters or leave them as flags for the driver?

@andrew-platt
Copy link
Collaborator

I think it makes sense to change the AeroDyn_Driver_Subs.f90 as well.

@bjonkman
Copy link
Contributor Author

AD driver has been updated as well.

@andrew-platt andrew-platt merged commit 5542dc9 into OpenFAST:dev Jun 16, 2023
@bjonkman bjonkman deleted the f/MHK_param branch June 16, 2023 03:16
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.

3 participants