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

Wrong argument passed to FAST_InitializeAll_T #2064

Closed
marchdf opened this issue Mar 1, 2024 · 5 comments · Fixed by #2121
Closed

Wrong argument passed to FAST_InitializeAll_T #2064

marchdf opened this issue Mar 1, 2024 · 5 comments · Fixed by #2121
Milestone

Comments

@marchdf
Copy link
Contributor

marchdf commented Mar 1, 2024

Bug description

The definition of the function FAST_InitializeAll_T has TurbId as it's second argument. The call sites: https://github.com/OpenFAST/openfast/blob/main/modules/openfast-library/src/FAST_Library.f90#L144 and https://github.com/OpenFAST/openfast/blob/main/modules/openfast-library/src/FAST_Library.f90#L148 pass in iTurb to that argument and not TurbId.

To Reproduce
I am calling openfast from amr-wind but this isn't specific to that and I don't build stand-alone openfast so I am not sure what the steps are to reproduce.

Expected behavior
I would expect TurbId to be passed to FAST_InitializeAll_T and not iTurb since that is what it expects in the argument list.

OpenFAST Version
3.5.2

System Information (please complete the following information):

  • OS: NAME="Rocky Linux" VERSION="8.9 (Green Obsidian)" ID="rocky" ID_LIKE="rhel centos fedora" VERSION_ID="8.9"
  • Compiler: GNU Fortran (Spack GCC) 10.4.0
@andrew-platt
Copy link
Collaborator

The problem appears to be due to a discrepency in how the TurbId is used in FAST.Farm vs with AMR-Wind. When used with FAST.Farm, the turbine id ranges from 1:NumTurbines, but when coupled to a C code, the turbine id ranges from 0:NumTurbines-1.

@andrew-platt
Copy link
Collaborator

Looking at the code, the FAST_Sizes routine does not have a variable TurbID. Are you referring to another call to FAST_InitializeAll_T from the FAST_OpFM_Init routine (https://github.com/OpenFAST/openfast/blob/main/modules/openfast-library/src/FAST_Library.f90#L589)?

For reasons completely unknown to me, the FAST_OpFM_Initroutine contains both TurbID and iTurb in its input argument list. I don't actually know why both arguments are there, or if they would ever be different. However the use of iTurb appears to be consistent in this routine -- it must match the index of the Turbine data structure getting passed to FAST_InitializeAll_T and all uses of the Turbine data structure that follows. So I propose we remove either TurbID or iTurb from the argument list in FAST_OpFM_Init as those appear redundant.

@marchdf
Copy link
Contributor Author

marchdf commented Mar 15, 2024

I am referring to this FAST_InitializeAll_T: https://github.com/OpenFAST/openfast/blob/main/modules/openfast-library/src/FAST_Subs.f90#L37. It uses TurbId in the argument list.

I don't know why there are 2 IDs being passed into FAST_OpFM_Init. In AMR-Wind we seem to pass a local and a global ID to those arguments: https://github.com/Exawind/amr-wind/blob/main/amr-wind/wind_energy/actuator/turbine/fast/FastIface.cpp#L224. I don't know why. Maybe @gantech or @psakievich have a historical perspective on this?

@bjonkman
Copy link
Contributor

Originally, Turbine was not an array, but there was a TurbID for file naming purposes. The iTurb variable was introduced in #20

@andrew-platt
Copy link
Collaborator

I'm going to close this issue in deference to the underlying issue outlined in #2096.

@andrew-platt andrew-platt linked a pull request Mar 26, 2024 that will close this issue
27 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants