Possible maxshift underestimate in aa_ddtr_strategy.cpp

[mmalenta]

Issue Summary When processing the data sent from Cheetah up to a DM of 1500, we see some spurious candidates at high DM values at the end of every chunk send to processing. These candidates are not in the same place when different chunk size is used and are always present at the end of the chunk, no matter the size.

Considering the highest maxshift is for the last DM range if (plan.user_dm(range-1).inBin > 1) { m_maxshift = (int) ceil(( ( str_dm[range-1].low + str_dm[range-1].step * m_ndms[range - 1] ) * m_dmshifts[nchans - 1] ) / ( tsamp )); m_maxshift = (int) ceil((float) ( m_maxshift + ( (float) ( SDIVINT*2*SNUMREG ) ) ) / (float) plan.user_dm(range-1).inBin) / (float) ( SDIVINT*2*SNUMREG ); m_maxshift = m_maxshift * ( SDIVINT*2*SNUMREG ) * plan.user_dm(range-1).inBin; if (( m_maxshift ) > m_maxshift_high) m_maxshift_high = ( m_maxshift ); } The second step m_maxshift = (int) ceil((float) ( m_maxshift + ( (float) ( SDIVINT*2*SNUMREG ) ) ) / (float) plan.user_dm(range-1).inBin) / (float) ( SDIVINT*2*SNUMREG ); seems to be underestimating the maxshift, by up to 400 samples in our test case. As it currently stands, this line adds the SDIVINT*2*SNUMREG padding to the maxshift before dividing by binning factor and then dividing the thing again by SDIVINT*2*SNUMREG to get the integer number of SDIVINT*2*SNUMREG.

I might be misunderstanding what is happening exactly in the dedispersion kernel, but my understating is that the binned time samples should be divided between a rounded-up integer number of SDIVINT*2*SNUMREG blocks. m_maxshift = (int)ceil(((m_maxshift / (float)plan.user_dm(range-1).inBin) ) + (float)(SDIVINT*2*SNUMREG)) / (float)(SDIVINT*2*SNUMREG)) should then correctly divide the already binned time samples amongst SDIVINT*2*SNUMREG blocks, with an extra, unbinned padding to ensure rounding up. Long story short: SDIVINT*2*SNUMREG padding should not be binned as in the original code.

[KAdamek]

I'm not sure as the addition of "( (float) ( SDIVINT2SNUMREG ) )" in: m_maxshift = (int) ceil((float) ( m_maxshift + ( (float) ( SDIVINT2SNUMREG ) ) ) / (float) plan.user_dm(range-1).inBin) / (float) ( SDIVINT2SNUMREG );

should prevent underestimation of maxshift.

Can you send us the file so we can investigate? This is on the current master?

[mmalenta]

https://github.com/AstroAccelerateOrg/astro-accelerate/blob/master/src/aa_ddtr_strategy.cpp line 94 (same code repeated in line 103).

I think the formatting made my explanation very confusing. What I'm proposing is moving ( 2 * SDIVINT * SNUMREG outside the bracket that gets divided by the binning factor. I think it should be only the maxshift divided by the binning factor and then padding of 2 * SDIVINT * SNUMREG added instead of maxshift plus padding divided by the binning factor.

[KAdamek]

Issue was resolved