ComputationalRadiationPhysics / jungfrau-photoncounter

Conversion of Jungfrau pixel detector data to photon count rate
GNU General Public License v3.0
2 stars 2 forks source link

Pedestal Drift Issues #48

Closed kloppstock closed 4 years ago

kloppstock commented 5 years ago

As discussed in issue #37, we tracked the change of the pedestal values (orange), the ADC input (black), the standard deviation of the pedestal updates (magenta) and the threshold for dark pixels / the sigma (red) for different pixels. Currently the threshold is five times the standard deviation and we will try to lower this significantly to about one time the standard deviation. Have we made any other significant mistakes when updating the pedestal map?

Cheers, Jonas

[1] The pixel (x=144,y=66): pedestal_updates_144_66

[2] The pixel (x=415,y=415): pedestal_updates_415_415

[3] The pixel (x=429,y=407): pedestal_updates_429_407

[4] The pixel (x=430,y=408): pedestal_updates_430_408

[5] The pixel(x=463,y=489): pedestal_updates_463_489

[6] The pixel(x=671,y=322): pedestal_updates_671_322

[7] The pixel(x=698,y=456): pedestal_updates_698_456

[8] The pixel(x=699,y=457): pedestal_updates_699_457

The drift map with the location of the pixels for reference: figure_12

sredford commented 5 years ago

Hi Jonas, thanks for the nice plots! This will make it much easer to discuss and understand the tracking.

Let's start by considering pixel [1]. The hit rate here is lower so the pedestal tracking should be easier.

Firstly, I'm concerned that the orange pedestal line doesn't sit symmetrically within the lowest level of black points. At the start it seems too low. By the end it's too high. Perhaps the pedestal calculation has too long a memory of the previous frames. How many frames memory does the rolling average use? I usually use 100.

Secondly, the envelope enclosed by the purple (pedestal +- std deviation) and red (pedestal +- threshold) lines is increasing over time. This makes me think that you define the threshold from the current standard deviation of the pedestal, which I believe is the wrong thing to do. The threshold should be defined from the standard deviation of the pedestal after the dark frames, and then not altered. Otherwise a bias is introduced. So, if you define the threshold as I do, the purple and red lines should stay a constant distance from the orange line, which is the only line that really moves.

For the other pixels, indeed it looks like you might be affected by high hit rate. With proper treatment of the threshold, the pedestal in these pixels should never update. Anyway, I suggest we fix the behaviour of pixel [1] first, and then look again.

Cheers, Sophie

kloppstock commented 5 years ago

Hello Sophie,

thank you for your feedback! Our pedestal calculation always uses the algorithm proposed by John D. Cook, so the memory covers all black values.

I will fix the standard deviation problem and set the threshold to 3 * stddev and then send new graphs.

Cheers, Jonas

kloppstock commented 5 years ago

Hello again,

I fixed the standard deviation and the exploding threshold. The pedestal calculation still uses John D. Cook's algorithm so it still has an "infinite" memory. Is this implementation sufficient?

Cheers, Jonas

pedestal_updates_144_66

pedestal_updates_415_415

pedestal_updates_429_407

pedestal_updates_430_408

pedestal_updates_463_489

pedestal_updates_671_322

pedestal_updates_698_456

pedestal_updates_699_457

The new (logarithmically scaled) drift map for reference: drift_map

sredford commented 5 years ago

Hi Jonas, It looks like you have fixed the thresholds now, good! But again considering just pixel [1], I would say the average is still being performed over too many frames.

Perhaps we are getting mixed up with the idea of 'infinite memory'. We do not desire an infinite memory, but due to the rolling nature of the calculation it's not possible to exactly remove the previous values. Only the previous average is removed.

The algorithm implemented in MovingStat.h has a rolling window size (n in MovingStat.h), for which I use 100 frames. What value do you use for that? For the first n frames (m_n < n), the next value is simply added to the average calculation (Add function in MovingStat.h) but for every further frame, the next value is added while the current value is subtracted (Push function in MovingStat.h). In this way the algorithm 'forgets' the past. This should allow the drift in [1] to be better followed than currently shown. Cheers, Sophie

heidemeissner commented 5 years ago

Hi Sophie,

thanks a lot for the information. We have a question concerning the update of the values. Up to now, Jonas and Florian planned to store the data for the window and subtract the value at the beginning, i.e. the value 100 or 1000 frames ago. The calculation time will be about the same, for the storage of the last 1000 values 1-3 MB would be required which is manageable. Would you prefer that we follow your approach? Which is better / more accurate / more stable?

Cheers, Heide

sredford commented 5 years ago

Hi Heide, It sounds like this is more correct than the estimation we make with the running average (we subtract the current average, you remember the previous 100 or 1000 frames and subtract the first one). This is fine, it should be more accurate. One question, the 1-3 MB is per pixel right? Also, how does your approach work for the medium and low gains? Cheers, Sophie and Carlos

TheFl0w commented 5 years ago

The 1-3 MB was a miscalculation on my side, we would actually need 32 + 2k bytes per pixel where k is the window size. It makes more sense to implement your approach for memory efficiency.

TheFl0w commented 5 years ago

Here is the algorithm I wanted to use to calculate the mean and variance within a given window size. In order to subtract the value that falls out of the window in the next step, we have to maintain an array of k values for a window size of k for every pixel. That would add up to 116 MiB for a window size of 100 already which becomes impractical for larger window sizes. We also need to synchronize the results of pedestal updates between GPUs so we'd have to copy all of that data every time we process a package of frames. I think your implementation in MovingStat.h makes things a lot easier. image

kloppstock commented 5 years ago

Hi,

I have another question concerning your continuous update algorithm. The code in the MovingStat.h file adds the the first n values up (during the calibration routine). How should this be handled if n and the number of calibration frames differ? My guess would be to sum up the calibration values using this "moving window". Am I correct in this assumption?

Cheers, Jonas

sredford commented 5 years ago

Hi Jonas,

There are two separate numbers to keep in mind: n - the size of the window the number of dark frames

The size of the window and the number of dark frames should be different. That way the average is sensitive to drifting. A typical window size is 100 frames, the smallest number of dark frames we consider is 1000, and this can increase to 10,000 if we want the detector to settle well.

As you say, for the first n frames the values are just added up. After that (again, as you say) the algorithm should transition to the 'moving' regime, where for every value added the average is forgotten. This transition should happen within the dark frames.

It doesn't make sense for the size of the window to be larger than the number of dark frames. How is the code setup? Does the user input both n and the number of dark frames to be processed without thresholding? In which case, could there be a check that n < number of dark frames?

Cheers, Sophie

kloppstock commented 5 years ago

Hi Sophie,

thank you for your quick reply! I am currently in the process of implementing this. I plan to make n and the number of dark frames user selectable at compile time. I will integrate a check that ensures that n < number of dark frames is always given.

Cheers, Jonas