On Tue, Sep 26, 2017 at 02:09:56PM -0400, wrote: > On Tue, Sep 26, 2017 at 07:30:40PM +0200, Andreas Tille wrote: > > Hi Marc, > > > > thanks for the quick response. > > > > On Tue, Sep 26, 2017 at 06:45:42PM +0200, Marc Rehmsmeier wrote: > > > Hi Andreas, > > > > > > I am not entirely sure what I was doing there and then. That seems to be > > > version 2.1.2, correct? > > > > Yes, that's correct. We always try to package the latest upstream > > > > > > > In version 2.1.1, the array is allocated with ALPHASIZE+1. The <= in the > > > k-loop is originally not a mistake; the function's sibling, > > > init_dl_dangle_dg_ar, has the <= in the first loop (the i-loop). These > > > are the allocations in version 2.1.1: > > > > > > double dr_dangle_dg_ar[ALPHASIZE][ALPHASIZE][ALPHASIZE+1]; > > > double dl_dangle_dg_ar[ALPHASIZE+1][ALPHASIZE][ALPHASIZE]; > > > > > > You see that there's a +1 on the corresponding sides. > > > > Yes, I see. Would you like to express that the better fix for that > > issue would be to restore this allocation inside energy.h instead of > > fixing the loop index? > > So someone broke the allocation size in 2.1.2? > > Seems like a valid fix too, especially if there is some actual use for > the extra slot. > > > > At some point I introduced an additional letter to the alphabet, the X (a > > > masking letter; see input.h). I am not sure about the program logic > > > anymore, whether X can actually be used for lookups in the energy tables > > > (as the N could). In hybrid_core.c, where the energy functions are used, > > > I check for X in a few places, so the idea was perhaps to not use X for > > > lookups (in fact, I do not want to consider any masked sequence at all). > > > Should that be true, ALPHASIZE being 6 would be wasteful, but it > > > shouldn't harm otherwise. Of course, you could change the two for-loops > > > (the k-loop in init_dr_dangle_dg_ar and the i-loop in its sibling) so > > > that they use < instead of <=, but, while that might fix the array bound > > > bug, it might not solve an underlying logic problem. In effect you might > > > have a program that works technically, but produces the wrong result. > > > Unfortunately I can't help with that. You could try fixing the loops and > > > then do a few runs and compare the outputs, as a minimal test. > > > > So it seems to me that it is safer to leave the loop untouched ... > > Makes sense if the energy.h is fixed instead.
Of course it seems the change was a number of places. ALPHASIZE was changed from 5 to 6, which removed the need for the +1 in a number of places, but of course makes the arrays a chunk larger. I think in fact just fixing the two places that initialize the array to 0 to not have the <= is in fact the safer option since that will match the allocation. Everywhere else that had to deal with the +1 seems to have already been changed in 2.1.2 to match ALPHASIZE now being 6. Of course the larger arrays might have something to do with why the compile takes a bit longer than before. But that could also just be a gcc change. -- Len Sorensen
