When testing the step 1-15 with fpmradld = 1.5, NBrings = 40 I got a core dump at line 5729 of PIAACMCsimul_exec() in PIAACMCsimul.c. I added some diagnostic messages, restarted from step 14 (optsingle 15) and the core dump did not occur and step 14 completed normally. I then removed my diagnostics, made sure I was not reinitializing conf (just in case), restarted from step 14 and the core dump did not recur and step 14 completed normally.

Here's what I see in the core dump.  optsyst does not seem to be well formed.
(lldb) bt
* thread #1: tid = 0x0000, 0x0000000100b11607 PIAACMCdesign`PIAACMCsimul_exec(confindex=, mode=) + 19623 at PIAACMCsimul.c:5729, stop reason = signal SIGSTOP
  * frame #0: 0x0000000100b11607 PIAACMCdesign`PIAACMCsimul_exec(confindex=, mode=) + 19623 at PIAACMCsimul.c:5729
    frame #1: 0x0000000100b18b53 PIAACMCdesign`PIAACMCsimul_run(confindex="piaacmcconf_i000", mode=) + 547 at PIAACMCsimul.c:9207
    frame #2: 0x0000000100b1a00a PIAACMCdesign`PIAACMCsimul_run_cli + 74 at PIAACMCsimul.c:185
(lldb) di -l
PIAACMCdesign`PIAACMCsimul_exec + 19612 at PIAACMCsimul.c:5729
   5728                     for(elem=0; elem: movq   %r9, %rdi
    0x100b115ff <+19615>: movl   $0xc, %ecx
    0x100b11604 <+19620>: movq   %r11, %rsi
->  0x100b11607 <+19623>: rep    
    0x100b11608 <+19624>: cmpsb  %es:(%rdi), (%rsi)
    0x100b11609 <+19625>: cmoveq %rdx, %rax
(lldb) p elem
(long) $0 = 145934


(lldb) p optsyst[0]
(OPTSYST) $1 = {
  nblambda = 0
  lambdaarray = {
    [0] = 0
    [1] = 1.3138771236713042E-314
    [2] = -1.824311067610789E+232
    [3] = -50.006507405026994
    [4] = 2.139652424518647E-255
    [5] = -8.6286370881233509E+193
    [6] = 2.5784379341662965E+203
    [7] = 7.6206403119219413E-307
    [8] = 2.3013134521161993E+23
    [9] = -6.6977449806482143E-158
    [10] = 1.578784827271505E-235
...
  beamrad = 2.1971436865616037E-314
  pixscale = 5.4807161623626676E-314
  size = 499
  DFTgridpad = 140464504743072
  NBelem = 4447068359
  name = {
    [0] = "?\nb0"
    [1] = ""
    [2] = "?
    [3] = ""
    [4] = " ??p?\x7f"
    [5] = "?"
    [6] = "??   \x01"
    [7] = "\x01"
    [8] = "?U\x01?"
    [9] = ""
    [10] = "\x12\x02"
    [11] = ""
    [12] = ""
    [13] = "?"
    [14] = "\x92?   \x01"
    [15] = "\x01"
    [16] = "???\x01"
    [17] = "\x01"
...

The current code base has globals defined in .h files, which causes duplicate symbol errors in the OS X version of gcc. Here is a recommended fix:

1. Move the following globals to CLIcore.c:
   pid_t CLIPID;
   char DocDir[200]; // location of documentation
   char SrcDir[200]; // location of source
   char BuildFile[200]; // file name for source
   char BuildDate[200];
   char BuildTime[200];

uid_t euid_real;
uid_t euid_called;
uid_t suid;

int TYPESIZE[9];

int C_ERRNO;

2. add "extern" to the declaration of these globals in CLIcore.h
   extern pid_t CLIPID;
   extern char DocDir[200]; // location of documentation
   extern char SrcDir[200]; // location of source
   extern char BuildFile[200]; // file name for source
   extern char BuildDate[200];
   extern char BuildTime[200];

extern int C_ERRNO; // C errno (from errno.h)

…
extern uid_t euid_real;
extern uid_t euid_called;
extern uid_t suid;

…

extern int TYPESIZE[9];

Local variable 'bestval' in PIAACMCsimul_exec is still not initialized
before it is printed into linoptval.txt. 'xsize' further up in same file
(line 3173) is similarly uninitialized before printing to stdout, in this
path through the code.

In linopt_imtools.c, line 1700 there is
double *params;
but then
params[0] = 0.0;

This is very dangerously zeroing an arbitrary memory location, since
params is not set. The uninitialized 'params' is also sent to linopt_imtools_opt_f(),
but currently not used there. Finally, the address of params is used to
initialize a field of gsl_multimin_function_fdf, which is almost
certainly not what you want.
opt_func.params = &params;

At COREMOD_memory.c line 5548:

pthread_t thread_savefits = (pthread_t) 0;

this is nonportable/dangerous as pthread_t is opaque.
Of more concern, further down this function there is
pthread_join(thread_savefits, (void**)&thread_savefits);
which is a dangerous cast that might overwrite arbitrary
memory through the double indirect. Why not use NULL?

There are two other globals that cause duplicate symbol errors on OS X's gcc. The solutions below are somewhat arbitrary but work.

1. in COREMOD_arith.c, and COREMOD_memory.c, change
   char errmsg[SBUFFERSIZE];
   to
   extern char errmsg[SBUFFERSIZE];
   which leaves the actual declaration in image_basic.c

2. in COREMOD_iofits.c, change
   char errormessage[SBUFFERSIZE];
   to
   extern char errormessage[SBUFFERSIZE];
   which leaves the actual declaration in an external library routine.

Alternatively, these globals could be given unique names in their files.

v1.2.19 won't compile until you remove some unresolved merge goop
from {image_basic.c, linopt_imtool.c, COREMOD_memory.c,
COREMOD_tools.c, and CLIcore.c}. Stuff like
<<<<<<< HEAD

fe7a818

From Chris: In the master branch, could you please remove lines {1604,1605}
and{1954,1955}from PIAACMCsimul.c :

...
1604: double *piaar0;
1605: double *piaar1;
...
1954: free(piaar0);
1955: free(piaar1);
...

These stack variables are never used. They are never initialized either,
so the free is directed at random memory, which just produced some
really nasty behavior that took a little bit of sorting out.

in PIAACMCsimul.c replace “if(WRITE_OK=1)” with “if(WRITE_OK==1)”?

In 1.2.33 I have been unable to run step 100 with 4 tmux processes. All but one such process always eventually seg faults in a call to save_fits in COREMOD_iofits.c. save_fits eventually leads to a call to fits_create_file, which returns fptr = 0, though no error is indicated in FITSIO_status. This has happened in several contexts with several file names.

Examining the file names, it's pretty apparent that multiple tmux processes are often trying to write to the same file name (there is no process ID in the passed file name). This is clearly not thread safe. I suppose that the files being written by each thread with the same file names are the same, but if one thread has the filename open for writing while another thread tries to open the same file, I'd expect a problem.

I'm currently trying the solution of having save_fits call save_fits_atomic, which is thread safe. This requires a little care with the filename, so the version of save_fits I'm trying is

 int save_fits(char *ID_name, char *file_name)
{
    char savename[1000];
    if (file_name[0] == '!')
        strcpy(savename, file_name+1); // avoid the leading '!'
    else
        strcpy(savename, file_name);
    
    return save_fits_atomic(ID_name, savename);
}

This seems to be working, though I won't know for sure until step 100 is complete.

Edit: the above fix is working: I have completed 4 cases of steps 100 and beyond, and have completed one case of steps 0 through 15 with this change.
This change makes the following two assumptions:

• the intent is to always overwrite existing files with the same name. The code can easily be modified to avoid this assumption
• every time a file is written with the same name it has the same contents. This is an assumption of the existing code base, and is not due to my change.

Here are a couple example core dumps using the old version of save_fits.

Steves-Mac-Pro:tests steve$ lldb --core /Cores/core.59757 
(lldb) target create --core "/Cores/core.59757"
warning: (x86_64) /Cores/core.59757 load command 235 LC_SEGMENT_64 has a fileoff + filesize (0xf2f8a000) that extends beyond the end of the file (0xf2f89000), the segment will be truncated to match
warning: (x86_64) /Cores/core.59757 load command 236 LC_SEGMENT_64 has a fileoff (0xf2f8a000) that extends beyond the end of the file (0xf2f89000), ignoring this section
Core file '/Cores/core.59757' (x86_64) was loaded.
(lldb) di -l
PIAACMCdesign was compiled with optimization - stepping may behave oddly; variables may not be available.
PIAACMCdesign`ffcrim + 62 at putkey.c:32
   31  	
-> 32  	    if (fptr->HDUposition != (fptr->Fptr)->curhdu)
   33  	        ffmahd(fptr, (fptr->HDUposition) + 1, NULL, status);
PIAACMCdesign`ffcrim:
->  0x1018bef3e <+62>: movl   (%r12), %esi
(lldb) bt
* thread #1: tid = 0x0000, 0x00000001018bef3e PIAACMCdesign`ffcrim(fptr=0x0000000000000000, bitpix=-32, naxis=3, naxes=0x00007fff5e410ff0, status=0x0000000109a7e218) + 62 at putkey.c:32, stop reason = signal SIGSTOP
  * frame #0: 0x00000001018bef3e PIAACMCdesign`ffcrim(fptr=0x0000000000000000, bitpix=-32, naxis=3, naxes=0x00007fff5e410ff0, status=0x0000000109a7e218) + 62 at putkey.c:32 [opt]
    frame #1: 0x00000001017ef89f PIAACMCdesign`save_fl_fits(ID_name="psfi0", file_name="!piaacmcconf_i000/psfi0_pt002.fits") + 799 at COREMOD_iofits.c:1039
    frame #2: 0x00000001017f0e1b PIAACMCdesign`save_fits(ID_name="psfi0", file_name="!piaacmcconf_i000/psfi0_pt002.fits") + 123 at COREMOD_iofits.c:1416
(lldb) p fptr
(fitsfile *) $0 = 0x0000000000000000
(lldb) up
frame #1: 0x00000001017ef89f PIAACMCdesign`save_fl_fits(ID_name="psfi0", file_name="!piaacmcconf_i000/psfi0_pt002.fits") + 799 at COREMOD_iofits.c:1039
   1036	            }
   1037	        }
   1038	
-> 1039	        fits_create_img(fptr, FLOAT_IMG, (int) naxis, naxes, &FITSIO_status);
   1040	        if(check_FITSIO_status(__FILE__,__func__,__LINE__,1)!=0)
   1041	        {
   1042	            fprintf(stderr, "%c[%d;%dm Error while calling \"fits_create_img\" %c[%d;m\n", (char) 27, 1, 31, (char) 27, 0);
(lldb) p fptr
(fitsfile *) $1 = 0x0000000000000000
(lldb) up
frame #2: 0x00000001017f0e1b PIAACMCdesign`save_fits(ID_name="psfi0", file_name="!piaacmcconf_i000/psfi0_pt002.fits") + 123 at COREMOD_iofits.c:1416
   1413	        atype = data.image[ID].md[0].atype;
   1414	        switch(atype) {
   1415	        case FLOAT:
-> 1416	            save_fl_fits(ID_name, file_name);
   1417	            break;
   1418	        case DOUBLE:
   1419	            save_db_fits(ID_name, file_name);
(lldb) p ID_name
(char *) $2 = 0x0000000101908555 "psfi0"
(lldb) p file_name
(char *) $3 = 0x00007fff5e411500 "!piaacmcconf_i000/psfi0_pt002.fits"




Steves-Mac-Pro:tests steve$ lldb --core /Cores/core.60308 
(lldb) target create --core "/Cores/core.60308"
warning: (x86_64) /Cores/core.60308 load command 183 LC_SEGMENT_64 has a fileoff + filesize (0xa5d0b000) that extends beyond the end of the file (0xa5d0a000), the segment will be truncated to match
warning: (x86_64) /Cores/core.60308 load command 184 LC_SEGMENT_64 has a fileoff (0xa5d0b000) that extends beyond the end of the file (0xa5d0a000), ignoring this section
Core file '/Cores/core.60308' (x86_64) was loaded.
(lldb) bt
PIAACMCdesign was compiled with optimization - stepping may behave oddly; variables may not be available.
* thread #1: tid = 0x0000, 0x000000010dc2bf3e PIAACMCdesign`ffcrim(fptr=0x0000000000000000, bitpix=-64, naxis=2, naxes=0x00007fff520a3e10, status=0x0000000115deb218) + 62 at putkey.c:32, stop reason = signal SIGSTOP
  * frame #0: 0x000000010dc2bf3e PIAACMCdesign`ffcrim(fptr=0x0000000000000000, bitpix=-64, naxis=2, naxes=0x00007fff520a3e10, status=0x0000000115deb218) + 62 at putkey.c:32 [opt]
    frame #1: 0x000000010db5be62 PIAACMCdesign`save_db_fits(ID_name="fpmza", file_name="!piaacmcconf_i000/fpm_zonea_s2_l0565_sr10_nbr032_mr300_minsag-02000_maxsag002000_fpmreg001000_ssr50_ssm0_Mirror_wb10.fits") + 322 at COREMOD_iofits.c:880
    frame #2: 0x000000010db5de3b PIAACMCdesign`save_fits(ID_name="fpmza", file_name="!piaacmcconf_i000/fpm_zonea_s2_l0565_sr10_nbr032_mr300_minsag-02000_maxsag002000_fpmreg001000_ssr50_ssm0_Mirror_wb10.fits") + 155 at COREMOD_iofits.c:1419
(lldb) p fptr
(fitsfile *) $0 = 0x0000000000000000
(lldb) up
frame #1: 0x000000010db5be62 PIAACMCdesign`save_db_fits(ID_name="fpmza", file_name="!piaacmcconf_i000/fpm_zonea_s2_l0565_sr10_nbr032_mr300_minsag-02000_maxsag002000_fpmreg001000_ssr50_ssm0_Mirror_wb10.fits") + 322 at COREMOD_iofits.c:880
   877 	        }
   878 	
   879 	
-> 880 	        fits_create_img(fptr, DOUBLE_IMG, naxis, naxes, &FITSIO_status);
   881 	        if(check_FITSIO_status(__FILE__,__func__,__LINE__,1) != 0)
   882 	        {
   883 	            fprintf(stderr, "%c[%d;%dm Error while calling \"fits_create_img\" %c[%d;m\n", (char) 27, 1, 31, (char) 27, 0);
(lldb) p fptr
(fitsfile *) $1 = 0x0000000000000000
(lldb) p FITSIO_status
(int) $4 = 0
(lldb) up
frame #2: 0x000000010db5de3b PIAACMCdesign`save_fits(ID_name="fpmza", file_name="!piaacmcconf_i000/fpm_zonea_s2_l0565_sr10_nbr032_mr300_minsag-02000_maxsag002000_fpmreg001000_ssr50_ssm0_Mirror_wb10.fits") + 155 at COREMOD_iofits.c:1419
   1416	            save_fl_fits(ID_name, file_name);
   1417	            break;
   1418	        case DOUBLE:
-> 1419	            save_db_fits(ID_name, file_name);
   1420	            break;
   1421	        case USHORT:
   1422	            save_sh_fits(ID_name, file_name);
(lldb) p ID_name
(char *) $2 = 0x000000010dc7503a "fpmza"
(lldb) p file_name
(char *) $3 = 0x00007fff520a4300 "!piaacmcconf_i000/fpm_zonea_s2_l0565_sr10_nbr032_mr300_minsag-02000_maxsag002000_fpmreg001000_ssr50_ssm0_Mirror_wb10.fits"

Line 1440 in COREMOD_iofits.c is incorrect, causing a core dump:
sprintf("mv %s %s", fnametmp, file_name);
should be
sprintf(command, "mv %s %s", fnametmp, file_name);

I'll test this fix overnight tonight.

The easiest solution is to declare all declarations of clock_gettime static in CLIcore.c, fft.c, COREMOD_memory.c, cudacomp.c, linopt_imtools.c, and COREMOD_tools.c.
In each file, change
int clock_gettime(int clk_id, struct timespec *t){
to
static int clock_gettime(int clk_id, struct timespec *t){

With v. 1.2.18 (soon to become 1.2.19) I'm still seeing the ID = -1 core dumps. This problem happens at least once somewhere pretty much every time I run the code from scratch. This one happened in step 1, and rerunning step 1 did not repeat the problem. So this is not deterministic behavior. The problem is that the line ID = image_ID is sometimes returning -1, only when called at line 818 of linopt_imtools.c. While ID = image_ID is called in many many places, this is the only place the code has crashed.

The problem is always in getting the ID of the image named "psfc0". I could not find where this image is created. image_ID will always return -1 if the named image does not exist.

Here is what the debugger tells me:

(lldb) bt
* thread #1: tid = 0x0000, 0x0000000101eea250 PIAACMCdesign`linopt_imtools_Image_to_vec(ID_name=<unavailable>, IDpixindex_name="pixindex", IDpixmult_name="pixmult", IDvec_name="imvect") + 64 at linopt_imtools.c:819, stop reason = signal SIGSTOP
  * frame #0: 0x0000000101eea250 PIAACMCdesign`linopt_imtools_Image_to_vec(ID_name=<unavailable>, IDpixindex_name="pixindex", IDpixmult_name="pixmult", IDvec_name="imvect") + 64 at linopt_imtools.c:819
    frame #1: 0x0000000101e9423f PIAACMCdesign`PIAACMCsimul_computePSF(xld=<unavailable>, yld=0, startelem=0, endelem=<unavailable>, savepsf=<unavailable>, sourcesize=0, extmode=<unavailable>, outsave=1) + 2639 at PIAACMCsimul.c:3958
    frame #2: 0x0000000101e9aa74 PIAACMCdesign`PIAACMCsimul_exec(confindex=<unavailable>, mode=0) + 18260 at PIAACMCsimul.c:5519
    frame #3: 0x0000000101ea2523 PIAACMCdesign`PIAACMCsimul_run(confindex="piaacmcconf_i000", mode=<unavailable>) + 547 at PIAACMCsimul.c:8739
    frame #4: 0x0000000101ea39da PIAACMCdesign`PIAACMCsimul_run_cli + 74 at PIAACMCsimul.c:185

(lldb) di -l
PIAACMCdesign`linopt_imtools_Image_to_vec + 48 at linopt_imtools.c:819
   818      ID = image_ID(ID_name);
   819      naxisin = data.image[ID].md[0].naxis;
   820      atype = data.image[ID].md[0].atype;
PIAACMCdesign`linopt_imtools_Image_to_vec:
    0x101eea240 <+48>: shlq   $0x4, %rbx
    0x101eea244 <+52>: movq   0x1298(%r13), %rax
    0x101eea24b <+59>: movq   0x10(%rax,%rbx), %rax
->  0x101eea250 <+64>: movq   0x50(%rax), %r15

(lldb) p ID
(long) $1 = -1

(lldb) up
frame #1: 0x0000000101e9423f PIAACMCdesign`PIAACMCsimul_computePSF(xld=<unavailable>, yld=0, startelem=0, endelem=<unavailable>, savepsf=<unavailable>, sourcesize=0, extmode=<unavailable>, outsave=1) + 2639 at PIAACMCsimul.c:3958
   3955 
   3956 
   3957  //           list_image_ID();
-> 3958             linopt_imtools_Image_to_vec("psfc0", "pixindex", "pixmult", "imvect");
   3959            
   3960            
   3961             save_fits("imvect", "!test_imvect.fits");

From Chris: I got a little more time this morning to look at the PIAACMCdesign code,
followed up on something that is seg faulting in step007 with agressive
optimization, and ran into some confusing code. I'm hoping you can
answer a few questions about it.

In PIAACMCsimul.c::PIAACMCsimul_exec() the following variables
are declared on the stack, and not initialized:

long i, ii, jj, kk;

The first part of the function is a giant branch, switching on 'mode'.
For some of the modes, variable ii is used in one or more loops, and
ends up with the loop termination value, e.g.

for(ii=0;ii<xsize_ysize_zsize; ii++) ...

For step007, mode=40 and ii is not set, so it gets a random stack
value. Then LINOPT==1, so we drop into that code next, where we
use jj as a loop variable, but assign into F[ii]

for(jj=0; jj<data.image[piaacmc[0].piaa0CmodesID].md[0].size[0]; jj++)
{
tmp = piaa0C_regcoeff * (data.image[ID].array.F[jj]-data.image[IDref].array.F[jj]) * pow(1.0_jj,piaa0C_regcoeff_alpha);
data.image[ID1Dref].array.F[ii] = tmp;
val0 += tmp_tmp;
}

Depending on ii, it will either seg fault here, or continue to uselessly
overwrite the same F[ii] location. There are 3 more jj loops with the
same assignment to F[ii], and many compiler optimization routines
will remove them, since if not fatal the assignment is idempotent.

There are lots more locations where this same pattern is repeated. Is
the [ii] just a typo? I thought this might be the case, but the next thing
the code does in step007 is to use ii as a loop variable

for(ii=0; ii<data.image[ID].md[0].nelement; ii++)

so it is left with value nelement (4872) at loop termination. Then there
are a bunch of loops that look like this

for(jj=0; jj<data.image[piaacmc[0].piaa0CmodesID].md[0].size[0]; jj++)
        {
            data.image[ID1Dref].array.F[ii] = piaa0C_regcoeff * (data.image[ID].array.F[jj]-data.image[IDref].array.F[jj]) * pow(1.0*jj, piaa0C_regcoeff_alpha);
            data.image[IDm].array.F[ii] = 1.0;
            ii++;
        }

Now we have a loop indexed by jj and assign into F[ii], but also increment ii,
so perhaps this is deliberate? If not deliberate, there are some crazy
assignments going on. If deliberate, this is incredibly confusing code.

From Chris: Here's a racy bug:

In COREMOD_memory.c, line 1547

   # ifdef _OPENMP
   #pragma omp atomic 
   #endif
   ID = next_avail_image_ID();

   data.image[ID].used = 1;

The 'atomic' is irrelevant, since ID is on the calling thread's
stack. (It is also illegal - needs to be 'atomic write'.)

However, the real problem is in next_avail_image_ID():

long next_avail_image_ID() /* next available ID number */
{
long i;
long ID = -1;
int found = 0;

ifdef _OPENMP

#pragma omp critical
{

endif

   for (i=0; i<data.NB_MAX_IMAGE; i++)
   {
       if((data.image[i].used == 0)&&(found == 0))
       {
           ID = i;
           found = 1;
       }
   }

ifdef _OPENMP

}

endif

if(ID==-1)
ID = data.NB_MAX_IMAGE;

return(ID);
}

The danger here is that the critical section is left before
data.image[ID].used is set, since that is left to the calling
thread, so multiple threads could get the same ID. Putting
the update inside the critical section (and adding a break
so it doesn't traverse the loop NB_MAX_IMAGE (=5000)
times) avoids the race.

ifdef _OPENMP

#pragma omp critical
{

endif

   for (i=0; i<data.NB_MAX_IMAGE; i++)
   {
       if ( data.image[i].used == 0 ) 
       {
           ID = i;
           data.image[ID].used = 1;
           break;
       }
   }

ifdef _OPENMP

}

endif

The 'atomic' and 'data.image[ID].used = 1;' should be removed
from the callers - the one mentioned up top, and another at
line 2141 (which doesn't have an 'atomic' to begin with).

Bad things are going to happen if data.NB_MAX_IMAGE is
ever returned.