Fix get_array() when introducing gaps in multi-module detector data

[takluyver]

@daviddoji alerted me to a bug when reading LPD data, where the same data would appear in the array twice.

This turned out to be when we introduce a gap in the data for one module, where that module missed a train which other modules recorded. I.e. a contiguous chunk of data in the file has to be split into 2 or more pieces in the output array. After each split, it was going back to the start of the source chunk, instead of continuing from after the split point. This only affects the new code path for reading with a pulse selection.

Edit: I found another bug when introducing gaps and not using pulse selection. Another win for tests.

I'm not exactly sure how to have a good test for this.

[takluyver]

Loading a single pulse from selected trains for all available LPD modules, then taking a single module and averaging each frame to get one number per train.

Before:

After:

The downward spike is the gap - because this is integer data (uncorrected), the gap is filled with zeros.

[takluyver]

Good job I did decide to write a test, because I immediately discovered another bug. 😅

[tmichela]

LGTM.

Any feel how often these gaps appear IRL? Do we need a bugfix release for that?

[takluyver]

Thanks! I think these gaps are reasonably common - @daviddoji noticed a couple of repeats by inspecting data from a single LPD module, and if there were a couple of fairly visible ones, chances are there were others that weren't so easy to see. So I'm planning to do a bugfix with this and Philipp's #236.

[takluyver]

I meant to say, though, I think people often want only trains with data for all modules (min_modules=16), in which case they can avoid this issue. So that mitigates it a bit. But unfortunately, the default when you use classes like LPD1M is to find trains with data from any module.

[takluyver]

David mentioned today that using min_modules to select only trains with data for all modules dropped about 600 trains from a big run of ~30000 trains. That implies that each module is missing about 1 train in 650 - or there might be some flaky modules and others more reliable. This is from data taken just a couple of weeks ago (p900226, r172).

[daviddoji]

I meant to say, though, I think people often want only trains with data for all modules (min_modules=16), in which case they can avoid this issue. So that mitigates it a bit. But unfortunately, the default when you use classes like LPD1M is to find trains with data from any module.

As we've already discussed, giving to min_modules a value greater than 1 would not be ideal. With LPD data, there is unfortunately bad luck in making all modules to work simultaneously, so one has to choose either using 12 or 13.

As one can decide how many of them would like to have in the analysis by using min_modules flag, I'll leave it as it is

@tmichela with long runs this happens quite frequently, unfortunately.