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

AD14: Restructure loop to fix optimization bug #348

Merged

Conversation

rafmudaf
Copy link
Collaborator

Complete this sentence
THIS PULL REQUEST IS READY TO MERGE

Feature or improvement description
When compiling with gfortran version 8+ on macOS and -O3 compiler optimization level, AeroDyn 14 seg faults. This seems to be an issue with the compiler as the faulty loop shouldn't be compiled since it has a read and write to a location in memory within the same iteration loop. In any case, this change removes the read-write operation in the same line by replacing the loops with a more explicit summation.

Related issue, if one exists
#347

Impacted areas of the software
AeroDyn 14

Additional supporting information
None

Test results, if applicable
As seen below, 4 tests did fail in the regression test suite. However, this is due to running with the newer gfortran compiler rather than gfortran-7 which was used to generate the baselines. These test cases show that the bug is fixed, and the error plots for the failing tests are consistent with what we expect when changing the compiler.

89% tests passed, 4 tests failed out of 37

Label Time Summary:
aerodyn14    = 664.15 sec*proc (7 tests)
aerodyn15    = 4426.03 sec*proc (19 tests)
beamdyn      = 504.81 sec*proc (11 tests)
dynamic      =  55.60 sec*proc (3 tests)
elastodyn    = 4709.51 sec*proc (25 tests)
hydrodyn     = 3373.92 sec*proc (7 tests)
linear       =  67.51 sec*proc (4 tests)
map          = 208.08 sec*proc (3 tests)
moordyn      = 176.10 sec*proc (1 test)
openfast     = 5156.39 sec*proc (29 tests)
servodyn     = 5151.77 sec*proc (26 tests)
static       =   2.33 sec*proc (4 tests)
subdyn       = 2989.74 sec*proc (3 tests)

Total Test time (real) = 2104.37 sec

The following tests FAILED:
	  9 - UAE_Dnwind_YRamp_WSt (Failed)
	 16 - SWRT_YFree_VS_WTurb (Failed)
	 19 - 5MW_OC3Trpd_DLL_WSt_WavesReg (Failed)
	 20 - 5MW_OC4Jckt_DLL_WTurb_WavesIrr_MGrowth (Failed)

Since there is a read and write in the same line, these do-loops shouldnt be optimization by the compiler. However, GNU Fortran version 8+ does optimize anyway leading to a seg fault.

This change removes the read-write operation in the same line by replacing the loops with a more explicit summation.
@rafmudaf rafmudaf self-assigned this Oct 22, 2019
@rafmudaf rafmudaf requested a review from andrew-platt October 22, 2019 18:44
Copy link
Collaborator

@andrew-platt andrew-platt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

I'm a bit surprised the optimizer was tripping on this one.

@andrew-platt andrew-platt merged commit de5b749 into OpenFAST:dev Oct 23, 2019
@rafmudaf rafmudaf deleted the bugfix/ad14_optimization_segfault branch October 23, 2019 15:24
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.

2 participants