search

ara-software / AraRoot

ARA data access framework
2 stars 7 forks source link

repeder updates #32

Closed MyoungchulK closed 2 years ago

MyoungchulK commented 2 years ago
  1. bug fix The median calculation is creating a negative value I believe h->GetXaxis()->GetBinCenter(i-1) is basically performing the equation that estimating median from histogram. The negative value can be happened by two cases a) When the frequency of the median group is close to zero, the estimation results -0.5. And floor() makes as -1 b) When there are no counts in the histogram, int get_median_slice() returns nan (zero division) based on median calculation. But the force integer conversion is causing an error. So, It is resulting typical huge minus number.

To fix the bug I implemented the condition that get_median_slice() to return 0 if median estimation is minus or nan. I also modified the mean calculation to return 0 if they encounter zero division

  1. can input quality cut results Based on the phone call talk (DB2530), some of the events are not suitable for pedestal production. Especially, the WF that is recorded while bias voltage feeds are unstable had an unphysical shape. In order to avoid these WFs. I add a quality cut option (-q) for selecting a good WF for pedestal production. -q option will accept the txt file that contains 0(bad) or 1(good) values for all events. If -q option is used, the repeder will use only good(1) events

Let me know if anyone has a question!:)

cozzyd commented 2 years ago

I looked at the changes to repeder.cc only and they look fine to me (fix the median/mean calculation in case of no events and add a quality file). Note that the median is not mean to be an estimation but should be exact (since the histogram is meant to include all possible ADC values... doing it with a histogram avoids the need to store all the values which could be large for a large number of events!).

MyoungchulK commented 2 years ago

I looked at the changes to repeder.cc only and they look fine to me (fix the median/mean calculation in case of no events and add a quality file). Note that the median is not mean to be an estimation but should be exact (since the histogram is meant to include all possible ADC values... doing it with a histogram avoids the need to store all the values which could be large for a large number of events!).

Thank you for the review! other parts are from the previous pull requests. somehow It is included because of my outdated branch. And yeah, I also agree histogram is the only way to avoid large memory usage (somehow, I was thought calculating median from histogram cannot be exact. my bad:)