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

Initilization error and VTK outputs prevents a clean abort #250

Closed
ebranlard opened this issue Feb 21, 2019 · 6 comments
Closed

Initilization error and VTK outputs prevents a clean abort #250

ebranlard opened this issue Feb 21, 2019 · 6 comments

Comments

@ebranlard
Copy link
Contributor

ebranlard commented Feb 21, 2019

Bug description

If an error occurs during the fast library initialization, the routine ExitThisProgram is called, which then attempts to write the mesh for debugging if WrVTK>0. The issue is that y_FAST%NOutSteps might be zero, and then the program will crash on the following statement Twidth = int(log10(real(y_FAST%NOutSteps))) + 1 .

To Reproduce

Try to trigger an error at initialization (e.g. by reproducing #249), and set WrVTK to on.

Expected behavior

The program should abort nicely.

Suggested solution

The calls to log10 in FAST_Subs.f90 should be protected as follows:

   if (y_FAST%NOutSteps==0) then
      CALL SetErrStat( ErrID_Warn,'NOutStep is zero, setting Twidth to 1',ErrStat2,ErrMsg2,RoutineName)
      Twidth = 1
   else
       Twidth = int(log10(real(y_FAST%NOutSteps))) + 1
   endif

I'll send a pull request later that fixes this if people agree.

@bjonkman
Copy link
Contributor

I think that looks good. I'm not sure the warning is necessary, though.

@ebranlard
Copy link
Contributor Author

ebranlard commented Mar 8, 2019

The pullrequest #257 solves this issue but I have now discovered a related bug:

To reproduce it, set WrVTK=2, VTK_type=1 and OutFileFmt=1. OpenFAST will then crash since NOutStep is 0, and the calculation of Twidth is wrong. For gfortran, the program will crash in ModMesh.f90 while trying to generate the file name.

The issue is that NOutStep is used for Twidth, but NOutStep is only defined when WrBinOutFile is on. I also believe that NOutStep should be different for VTK files, based on the n_VTKTime for instance.

@ebranlard
Copy link
Contributor Author

ebranlard commented Mar 8, 2019

I'm suggesting the following changes in FAST_Subs.f90, routine FAST_InitOutput:

  • Always compute NOutSteps, not only when WrBinOutFile is true, OR, initialize it to 1. It needs initialization since gfortran for instance will not initialize it to 0.

  • After NOutSteps is set, take the maximum between this value and the number of outputs required by the VTK_fps parameter:

   REAL(ReKi)                       :: DT_OUT_VTK                                      ! Output rate for VTK animations
   ![...]
   y_FAST%NOutSteps = 1 ! Initialization
   ![...]
   IF (p_FAST%n_VTKTime>0) THEN
       ! VTK animation may required more OutSteps, so we take the max
       DT_OUT_VTK = p_FAST%n_VTKTime * p_FAST%DT
       y_FAST%NOutSteps = MAX ( y_FAST%NOutSteps, CEILING ( (p_FAST%TMax - p_FAST%TStart) / DT_OUT_VTK ) + 1 )
   ENDIF

I'll submit another pull request if you agree with these changes.

NOTE: @jjonkman this also affects FAST.Farm

ebranlard added a commit to ebranlard/openfast that referenced this issue Mar 8, 2019
ebranlard added a commit to ebranlard/openfast that referenced this issue Mar 8, 2019
@bjonkman
Copy link
Contributor

I don't think NOutSteps should be used in the calculation for tWidth at all... The VTK files are written at a rate independent from the rate the normal FAST output files are written, and the VTK output start at t=0, not TStart.

In my version of the code, I use

tWidth=CEILING( log10( real(p_FAST%n_TMax_m1+1, ReKi) / p_FAST%n_VTKTime ) ) + 1

This is actually what I had suggested in the first issue reported in #172, and which I thought had already been fixed.

@ebranlard
Copy link
Contributor Author

Thanks for pointing out #172, it is indeed related to this second issue I mentioned. Unfortunately, it seems the change you suggested are not included in the current dev branch.

I agree that the rates are different, my solution was more of a quick fix. Probably the best would be to pass twidth as an argument to the write functions, but as long as these functions are only used for the visualization, it seems fine.

I can submit a pull request that include your version of twidth, wrapping it with an if statement to avoid any division by zero or log10 of 0.

ebranlard added a commit to ebranlard/openfast that referenced this issue Mar 12, 2019
rafmudaf added a commit that referenced this issue Mar 18, 2019
Zero padding of VTK names, and log10 of zero (see #250)
@rafmudaf
Copy link
Collaborator

closed by pull request #257

ashesh2512 pushed a commit to ashesh2512/openfast that referenced this issue Mar 21, 2019
ashesh2512 pushed a commit to ashesh2512/openfast that referenced this issue Mar 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants