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

BeamDyn: Updated integration scheme for nodal load outputs #163

Closed
wants to merge 7 commits into from

Conversation

bjonkman
Copy link
Contributor

@bjonkman bjonkman commented Sep 5, 2018

@andrew-platt updated the integration scheme he used for the nodal load outputs in BeamDyn. The updated integration scheme better matches the analytical result:
regtest01_internalforcecalcimprove

@ashesh2512
Copy link
Contributor

There are 3 minor merge conflicts resulting from the recently merged pull request #155. I have gone through the conflicts and once this pull request is approved, I should easily be able to resolve those conflicts before merge.

@ashesh2512
Copy link
Contributor

ashesh2512 commented Sep 7, 2018

@bjonkman overall the code looks good to me. I have a few comments -

  1. Following pull request BeamDyn finite differencing capability and variational consistent output in summary file #155 the error calls at lines 3657 and 3663 can very well use RETURN instead of CYCLE because the calling function (BD_Static) now checks for errors and aborts immediately upon calling BD_StaticSolution. Would you agree ?

  2. In the subroutine BD_diffmtc, the data structure p is used only to access nodes_per_elem. If so, I suggest passing in nodes_per_elem instead of p. This would make that routine modular and hence unit-test-able.

  3. Do you know if any of the regression test results are affected as a result of the update integration scheme ?

  4. Do you (or @andrew-platt) have any documentation regarding the computation of BldInternalForceFE ? Adding this information to the BeamDyn manual might be very helpful for new developers of OpenFAST/BeamDyn.

@bjonkman
Copy link
Contributor Author

@ashesh2512

I don't have time to address about all of your comments today, but I did want to mention something regarding the error handling in your first comment: Your static algorithm is a little different than ours, but I do not think that #155 should have added a return after the call to BD_StaticSolution. Ignoring the error is intentional there (i.e., the lines 3512-3516 that you added to BeamDyn.f90 were intentionally left out) otherwise the else branch in the statement that follows is irrelevant. You could perhaps change the if (piter .le. p%niter) then to if (ErrStat2 < AbortErrLev) instead, but the error checks after BD_StatisSolution shouldn't occur.

Also, the lines you added at the end of that routine (lines 3562-3566 in BeamDyn.f90) are redundant given the two lines that follow them.

I think there are some other potential error handling issues in your BD_StaticSolution subroutine in #155 (again, related to returning on error with the loop counter less than the max specified in that loop).

@ashesh2512
Copy link
Contributor

@bjonkman

The else condition still works because I removed line 3746 in BD_StaticSolution in #163 . I believe that line wasn't significant anymore considering the fact when (piter .le. p%niter) we reduce the load and move on till the maximum number of load steps is reached. So we never really exit the code for a static simulation based on whether the maximum Newton steps have been reached.

I agree with the redundancy of lines 3562-3565. I will submit a pull request for that right away. Thank you for pointing it out.

@ashesh2512
Copy link
Contributor

This pull request has been modified in #265 due codes diverging significantly.

@ashesh2512 ashesh2512 closed this Mar 27, 2019
ashesh2512 added a commit that referenced this pull request Mar 27, 2019
…puts) (#265)

* Updated BD linearizaon perturbation precision

* updated error handling in static solve

* BD: fixed assumption about first-node relative rotation

* BD bug fix: point load array size with multiple elements

* BD bug fix: removed extra GlbRot transform on output QP loads

output forces -- removed extra GlbRot transform on output QP forces / moments

* BD bug fix: Updated integration of nodal output loads

New integration implemented by @andrew-platt.

* Fixes for multi-element load outputs at nodes

* clean up a few comments and modify some code logic to align with the dev branch after merge from Envision

* Merge conflict cleanup

* Update the BeamDyn reg test baselines

* Add more outputs in BeamDyn 5MW reg test case
rafmudaf pushed a commit to rafmudaf/openfast that referenced this pull request Apr 8, 2019
…load outputs) (OpenFAST#265)

* Updated BD linearizaon perturbation precision

* updated error handling in static solve

* BD: fixed assumption about first-node relative rotation

* BD bug fix: point load array size with multiple elements

* BD bug fix: removed extra GlbRot transform on output QP loads

output forces -- removed extra GlbRot transform on output QP forces / moments

* BD bug fix: Updated integration of nodal output loads

New integration implemented by @andrew-platt.

* Fixes for multi-element load outputs at nodes

* clean up a few comments and modify some code logic to align with the dev branch after merge from Envision

* Merge conflict cleanup

* Update the BeamDyn reg test baselines

* Add more outputs in BeamDyn 5MW reg test case
rafmudaf pushed a commit to rafmudaf/openfast that referenced this pull request Apr 9, 2019
…load outputs) (OpenFAST#265)

* Updated BD linearizaon perturbation precision

* updated error handling in static solve

* BD: fixed assumption about first-node relative rotation

* BD bug fix: point load array size with multiple elements

* BD bug fix: removed extra GlbRot transform on output QP loads

output forces -- removed extra GlbRot transform on output QP forces / moments

* BD bug fix: Updated integration of nodal output loads

New integration implemented by @andrew-platt.

* Fixes for multi-element load outputs at nodes

* clean up a few comments and modify some code logic to align with the dev branch after merge from Envision

* Merge conflict cleanup

* Update the BeamDyn reg test baselines

* Add more outputs in BeamDyn 5MW reg test case
@bjonkman bjonkman deleted the b/BD-loads branch July 19, 2019 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants