Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change rounding in PMT simulation #726

Merged
merged 9 commits into from
Jul 24, 2020

Conversation

andLaing
Copy link
Collaborator

Changes the way the output of the PMT simulation used in diomira is converted to integer
to better simulate the ADC.

This change should remove or significantly improve the ~15% loss of signal observed
in passing MC through the diomira city.

@andLaing andLaing requested a review from mmkekic June 18, 2020 16:41
@andLaing
Copy link
Collaborator Author

These changes were implemented in discussion with Vicente Herrero where we agreed that the round functionality better represented the actual functionality of the PMT ADC. See attached plot.
kibcoanaboodncme

@andLaing andLaing force-pushed the pmt-simulation-checks branch from f4c75e1 to a1d3b66 Compare July 7, 2020 15:13
@andLaing
Copy link
Collaborator Author

andLaing commented Jul 7, 2020

We found some other problems in the electronics simulation that caused loss of charge.

Specifically, the daq_decimate function was using an order 30 Chebyshev filter which affected the DC part of the signal, effectively reducing the magnitude. We have changed the function to use the scipy default which, while a bit slower, does not have a noticeable effect on the integrated signal. We plan to study diomira for bottlenecks in any case.

In order to set the ADC central values correctly we ran tests using files with 1, 2, 10, 20. 50 & 100 pe in various forms (1 bin, 2 bin, 5 bin,...) and checked that, without gain dispersion nor charge fluctuation, what value we got for the signal and that the other charge levels were the appropriate multiple of that value.

There is one test which I have marked skip, fee_test.test_spe_to_adc, this is an old test with a lot of hardwired numbers which tests a function which doesn't seem to be used in any other part of the code and simulates the electronics with the functions in an order which differs with respect to the sequence used in diomira. How should I deal with it? Change it to be more similar to the current paradigm or are we happy to remove the test?

@andLaing andLaing requested a review from jjgomezcadenas July 7, 2020 15:22
@andLaing
Copy link
Collaborator Author

comp_PR726.pdf
Here's a pdf with some plots comparing the input/output for the simplified electronics simulation (pmtblr) for the current master and this PR.

Similar reduction in systematics are seen comparing to the irene selection with more complicated distribution due to electronics noise and the full simulation and deconvolution steps.

@andLaing andLaing force-pushed the pmt-simulation-checks branch from a1d3b66 to 7e48fae Compare July 22, 2020 14:36
Copy link
Collaborator

@mmkekic mmkekic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the relevant changes in algorithm were approved by VH I can only review the code.
Can you squash:

  • Update single_pe tests for new decimate, Mark test_diomira_reproduces_singlepe veryslow, Test to show loss of single pe signal in pmt simulation;
  • Update description of marker, Add functions to run very slow tests with commandline option
  • Use new file in test_diomira_exact_result, New comparison file for test_diomira_exact_result

and I am happy to approve

@andLaing andLaing force-pushed the pmt-simulation-checks branch from 7e48fae to a262b9a Compare July 24, 2020 15:28
Copy link
Collaborator

@mmkekic mmkekic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR fixes ~15% signal loss after passing through diomira. The main changes are:

  1. using round instead of floor for adc converter simulation (a test is added to show that for each PMT the mean ADC is equivalent to the gain)
  2. change daq_decimate to use scipy default (a test is added to show that the signal is maintained after the down-sampling)
  3. change ADC central values
    All the algorithm changes are discussed and approved by V. Herrero. Good job Andrew in recovering our MC signal!

@carmenromo carmenromo merged commit 62958eb into next-exp:master Jul 24, 2020
@andLaing andLaing deleted the pmt-simulation-checks branch July 24, 2020 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants