AeroDyn/UnsteadyAero_Driver: Fix for bug #1346

[RiekeJ]

This pull request fixes a bug in AeroDyn/UnsteadyAero_Driver:

bug #1346: If SimMod==1 (reduced frequency model) is used, non-allocated variables are handed over to subroutines with intent(in), which causes a runtime error.

Feature or improvement description
Fix for bug #1346, added some if clauses and making some arrays 'optional'.

Related issue, if one exists
none

Impacted areas of the software
AeroDyn -> UnsteadyAero_Driver.f90

Additional supporting information
UnsteadyAero_Driver is only used for stand-alone run of a single airfoil unsteady aerodynamics tests.

[andrew-platt]

There may be some additional logic required for the optional inputs. I don't know this code very well. @bjonkman could you take a look at it?

[ebranlard]

The changes looks great, thanks for doing that!
I have minor comments below.

[ebranlard]

As a side note, we currently don't have r-tests for unsteady aerodynamics. I think it's because it requires a recompile of OpenFAST with -DUA_OUTS=ON. Now that we have a split compilation/testing process on github action, we could do that!

[ebranlard]

Thank you for doing the changes and fixing this bug!

(we indeed use 3 spaces for indentation. As for indenting the comments and the "error checks" 3 extra spaces, that's not my favorite style, but it is present elsewhere in the code and has been adopted by other developers.).

[bjonkman]

ErrStat should be set to ErrID_Fatal here. All of the error handling in OpenFAST assumes that errors are values > 0, and it's best practice to use the error level parameters.

Also, ErrStat and ErrMsg are not initialized in the other branches of the nested IF statement. So, this will result in unpredictable results on some compilers.

[bjonkman]

Based on the error message below, I think you want to use all() instead of any() here.

[RiekeJ]

Hi,
Bonnie, you're right. The fix is not complete nor fully correct.

We need to add:

@@ -304,7 +305,20 @@ subroutine setUAinputs(n,u,t,dt,dvrInitInp,timeArr,AOAarr,Uarr,OmegaArr)
304 +   ErrStat   = ErrID_None           ! no error has occurred
305 +   ErrMsg    = ""

And further correct:

@@ -341,10 +341,104 @@ subroutine setUAinputs(n,u,t,dt,dvrInitInp,timeArr,AOAarr,Uarr,OmegaArr)
341 -   if (any( (/ allocated(timeArr),allocated(AOAarr),allocated(OmegaArr),allocated(Uarr) /) )) then
341 +   if (all( (/ allocated(timeArr),allocated(AOAarr),allocated(OmegaArr),allocated(Uarr) /) )) then

And also:

@@ -359,13 +359,33 @@ subroutine setUAinputs(n,u,t,dt,dvrInitInp,timeArr,AOAarr,Uarr,OmegaArr)
359 -   errStat = -1
359 +   errStat = ErrID_Fatal

BR,
Johannes

[ebranlard]

We have now reverted this pull request.

@RiekeJ can you redo your pull request? If possible, chose "dev" as the target branch for your pull request (I didn't notice, but I've merged this pull request into main. I should have spotted that.).

[andrew-platt]

@RiekeJ, our apologies for the mixup on our end with merging.

The corrections you posted above look good!

[RiekeJ]

Hi,
now I'm a little bit confused. How can I help now. Should I create a new pull request? Unfortunately, I had already deleted my fork of OpenFAST, but I can fork it again and can create a new branch + pull request. Let me know, please, how to proceed.

[andrew-platt]

@RiekJ, I'm sorry about all the confusion here. We had a mixup on our end with the merge. I recreated the PR using your commit, and added the modifications from your comment above. See #1356

Again, my apologies for the issues on our end. And many thanks for contributing your code improvements!

[RiekeJ]

@andrew-platt no worries. I'm happy, that I could contribute here a tiny bit. Thanks for having OpenFAST, which is really a great tool.