Initialize ED RtHS to zeros #2317
Conversation
Observation -- read file ./tmp.dat using ProcessComFile inside the ReadBladeFile, and don't keep the data - at 64 lines, frequencies are ok and no huge acceleration values in OP - at 65 lines, something goes wrong and the frequencies change and huge accelerations in OP show up Possibility 1: Maybe the FileInfo%Lines is getting allocated to the stack even thought it should be on the heap at 65 lines? This would result in clobbering something behind it on the stack that was already there? Not sure that logic holds Possibility 2: At this size, it takes up enough space on the heap that when deallocated, something uninitialized ends up using that memory. Problems with this include the "why has this never been observed before" question.
…ddressing or null pointer dereferencing - CountWords would scan past end of line - ProcessComFile Cleanup would dereference a null pointer when getting NextFile pointer - WrScr would hide a character string allocation which caused a memory issue in ifx
Some of these values were not getting zeroed out. This was occasionally leading to spurious root acceleration values when memory that was previously occupied by something else non-zero was used.
If a file of more than 64 lines was read in, then discarded, whatever was on line 65 or later could end up in arrays used by RtHS (gcc 12.3.0, double precision release). This would show up in linearization with case `5MW_Land_Linear_Aero`.
|
This should be backported to 3.5.4 |
Also moved the zeroing into the `else` part of the error checking instead of after -- we could potentially have triggered memory violations otherwise and not gotten our error back.
|
Do you know which arrays were using uninitialized values? FAST 7 used to be built with a compiler option to initialize all variables to 0 (or false), but we were trying to make sure we explicitly initialized variables that need it. This should also give some performance improvements on variables that don't need to be initialized before their use. |
| NextFile => CurrFile%Next | ||
| DO | ||
| DO WHILE(ASSOCIATED(currFile)) | ||
| NextFile => CurrFile%Next |
There was a problem hiding this comment.
I am hopeful that this fixes some of the weird behavior that was sometimes causing regression tests to fail on file reads when the OpenFAST input file reading was changed to use this routine!
There was a problem hiding this comment.
I hope it improves the stability. This is definitely in the realm of undefined behavior, so it could have been doing anything when CurrFile was not associated
|
I don't know exactly which arrays weren't getting initialized. I had started looking into that, but then decided to just take brute force approach to make sure everything got initialized. |
I'm just a little concerned that some of those variables may need to be explicitly zeroed out in the RHS routine so they don't retain values between time steps. If the issue is in an initialization routine that's a little different. |
|
That's a very good point! I had been assuming it was an initialization issue only, but since it is in the RHS they may need explicit zeroing at every timestep as well. |
Ready to merge
Feature or improvement description
The
RtHSderived type in ElastoDyn contains many arrays, none of which were initialized to zero during init. Some of these arrays end up getting used without initializing, so whatever leftover stuff was in memory would then be accessed as values in some of these arrays.We discovered this issue while converting the parsing of the blade file to use the
FileInfodata structure and parsing. If a file of 64 lines long was loaded with theProcessComcommand then discarded without use during theReadBladeFileroutine, the5MW_Land_Linear_Aerocase would give the expected results (tested with gcc 12.3.0, double precision release). However, if more lines were present, whatever was written in those memory locations would end up in anRtHSarray. We noticed this as incorrect acceleration values for blade 1 nodes in they_oparray with values upwards of1e+294, which caused issues with linearization.If line 65 contained
r, the resulting frequencies would be:If line 65 contained
rrrrrrrr, the resulting frequencies would be off as follows:Explicitly setting all
RtHSarrays to0.0_ReKiduring allocation would get rid of this issue.@deslaughter also found an issue in the
ProcessComroutine that caused an index issue in Intel compilations.Related issue, if one exists
There may have been some phantom issues caused by this.
Impacted areas of the software
ED calculations of blade 1 acceleration values, and perhaps other issues.