Skip to content
This repository has been archived by the owner on Jun 5, 2024. It is now read-only.

SPE spec bug fix, adding trigger truth #283

Merged
merged 10 commits into from
Dec 10, 2021
Merged

SPE spec bug fix, adding trigger truth #283

merged 10 commits into from
Dec 10, 2021

Conversation

zhut19
Copy link
Contributor

@zhut19 zhut19 commented Dec 6, 2021

What is the problem / what does the code in this PR do

  • Fixing a bug in the SPE spectrum, we were only using channel 0 for nT.
  • Also adding trigger truth to the truth values, including
    • n_photon_trigger: number of photons that should pass the ADC trigger assuming no noise fluctuation; this also considers the possibility of one photon generating 2 PE and thus having a higher chance of passing ADC threshold.
    • n_pe_trigger: number of photons + dpe that should pass the ADC trigger assuming no noise fluctuation
  • Adding a new field raw_area: Sum of the gain per PE triggered
  • Those three fields has a copy of bottom only values

@zhut19
Copy link
Contributor Author

zhut19 commented Dec 6, 2021

Hi @JoranAngevaare, I am adding the required fields including n_photon_trigger to the truth output. But also realized there's a pretty obvious bug that for nT, we were using only the channel[0] single photon electron spectrum for all channels.

Copy link
Contributor

@FaroutYLq FaroutYLq left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this PR! This is really helpful.

wfsim/core/pulse.py Show resolved Hide resolved
wfsim/core/pulse.py Outdated Show resolved Hide resolved
wfsim/core/pulse.py Show resolved Hide resolved
Copy link
Contributor

@JoranAngevaare JoranAngevaare left a comment

Choose a reason for hiding this comment

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

Thanks Tianyu for this PR! Much appreciated. One piece of the code I don't fully follow, can you help me a bit?

wfsim/core/pulse.py Show resolved Hide resolved
wfsim/core/pulse.py Outdated Show resolved Hide resolved
wfsim/core/pulse.py Outdated Show resolved Hide resolved
wfsim/core/pulse.py Outdated Show resolved Hide resolved
wfsim/core/rawdata.py Outdated Show resolved Hide resolved
@zhut19 zhut19 marked this pull request as ready for review December 7, 2021 15:57
@petergaemers petergaemers merged commit e80adb0 into master Dec 10, 2021
@petergaemers petergaemers deleted the extra_truth branch December 10, 2021 10:22
@JoranAngevaare
Copy link
Contributor

I tested it and it really works great, thanks again Tianyu!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants