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

Added Sort for peakpos based on ADC energy value #134

Draft
wants to merge 1 commit into
base: co60
Choose a base branch
from

Conversation

RyutaroMatsumoto
Copy link
Collaborator

In simple_calibration.jl added Sort for peakpos based on ADC energy value to avoid sorting malfunction, which sometimes happens and leads to wrong calibration results.

Copy link
Contributor

@fhagemann fhagemann left a comment

Choose a reason for hiding this comment

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

Thanks for this PR.
Some comments from my side:

  • could you replace @info with @debug so that the logging is shown in debug mode only (or add a verbose keyword to omit the @info output)?
  • is this the correct place to put it? I somehow feel like this should go directly into RadiationSpectra applied to the output of peakfinder before returning it (?). This way, everybody using RadiationSpectra.peakfinder could profit from this.

Looking at LEGEND analysis:

  • @theHenks would this interfere with finding the Tl208 FEP vs. finding the pulser peak if we sort them by ADC?

@LisaSchlueter LisaSchlueter marked this pull request as draft March 13, 2025 19:24
@LisaSchlueter
Copy link
Contributor

Thanks for this PR. Some comments from my side:

  • could you replace @info with @debug so that the logging is shown in debug mode only (or add a verbose keyword to omit the @info output)?
  • is this the correct place to put it? I somehow feel like this should go directly into RadiationSpectra applied to the output of peakfinder before returning it (?). This way, everybody using RadiationSpectra.peakfinder could profit from this.

Looking at LEGEND analysis:

  • @theHenks would this interfere with finding the Tl208 FEP vs. finding the pulser peak if we sort them by ADC?

Hi, this for the feedback. This is work in progress and will not be merged like that into co60

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

Successfully merging this pull request may close these issues.

3 participants