Generic simple_calibration #108

LisaSchlueter · 2024-12-16T22:21:34Z

add a more generic simple_calibration function that can be accessed with keyword calib_type == :gamma which is useful for example for Co-60 data
add a minimalistic gamma peak-shape, that contains only Gaussian signal and flat background to fit low-statistics data

fhagemann · 2024-12-16T22:41:38Z

src/simple_calibration.jl

+    e_simple = e_uncal .* c
+    e_unit = u"keV"
+    # get peakhists and peakstats
+    peakhists, peakstats, h_calsimple, bin_widths = get_peakhists_th228(e_simple, gamma_lines, window_sizes; binning_peak_window=binning_peak_window, e_unit=e_unit)


get_peakhists_th228 also works for generic simple_calibration_gamma?
Or would that also need a get_peakhists_gamma?

Yes it does.
In principle we could consider re-writing/re-naming all ..._th228 calibration functions to be more generic for other gamma spectra. Some don't even need any modification at all and work out of the box for other sources.
I didn't do that to keep the changes as minimal as possible.

It makes sense to rename it, but maybe this could go along then with a PR onto the dataflow where we do a search and replace for these functions.

I agree. I can take care of this beginning of next year.

In that context, I'd suggest to change the julia energy-calibration metadata as well:

Add a field "source" in the metadata, which is "th228" for LEGEND-200, but can also be something else. Like that the dataflow can pick the correct gamma_lines and gamma_names from the energy calibration config.

OR

Rename "th228_lines" etc in the metadata to "gamma_lines".

I prefer option 1, because this allows you to switch easily back and fourth between different calibration sources.

src/simple_calibration.jl

src/specfit.jl

LisaSchlueter · 2024-12-16T23:20:44Z

Is this for Co60 or Co56?

I wrote and tested this for my local Co60 data, but it should work for any other calibration spectra, because the anticipated gamma energies are passed as an argument.

codecov · 2024-12-16T23:33:12Z

Codecov Report

Attention: Patch coverage is 65.85366% with 28 lines in your changes missing coverage. Please review.

Project coverage is 28.82%. Comparing base (6eece2d) to head (cace095).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
ext/LegendSpecFitsRecipesBaseExt.jl	0.00%	11 Missing ⚠️
src/simple_calibration.jl	82.05%	7 Missing ⚠️
src/chi2fit.jl	50.00%	2 Missing ⚠️
src/fit_calibration.jl	0.00%	2 Missing ⚠️
src/pseudo_prior.jl	0.00%	2 Missing ⚠️
src/specfit.jl	90.90%	2 Missing ⚠️
src/specfit_functions.jl	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #108      +/-   ##
==========================================
+ Coverage   28.39%   28.82%   +0.43%     
==========================================
  Files          37       37              
  Lines        3564     3594      +30     
==========================================
+ Hits         1012     1036      +24     
- Misses       2552     2558       +6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

theHenks

In general, the PR looks good. Thanks @LisaSchlueter .
My comments are mainly about the naming.
Do we wanna stick to the isotope naming or start having more generic names?

theHenks · 2024-12-17T14:50:46Z

ext/LegendSpecFitsRecipesBaseExt.jl

@@ -490,6 +490,41 @@ end
    end
 end

+@recipe function f(report::NamedTuple{(:h_calsimple, :h_uncal, :c, :peak_guess, :peakhists, :peakstats)}; cal=true)


I don't understand why this is a new recipe. Was this not here already before?

This is almost a duplicate of the existing recipe. However, the old one has some stuff hardcoded, e.g. "FEP" in the plot label. I didn't want to mess with the existing one, since we're int he middle of the processing.

theHenks · 2024-12-17T14:51:25Z

ext/LegendSpecFitsRecipesBaseExt.jl

@@ -820,7 +855,8 @@ end
        if plot_ribbon
            ribbon := uncertainty.(report.f_fit.(0:1:1.2*value(maximum(report.x))))
        end
-        0:1:1.2*value(maximum(report.x)), value.(report.f_fit.(0:1:1.2*value(maximum(report.x))))
+        xstep = value(maximum(report.x))/100


Does this also work for the 228-Th data

I tested it today on 228-Th data and it worked well, same result.

src/simple_calibration.jl

theHenks · 2024-12-17T14:54:54Z

src/simple_calibration.jl

+    e_simple = e_uncal .* c
+    e_unit = u"keV"
+    # get peakhists and peakstats
+    peakhists, peakstats, h_calsimple, bin_widths = get_peakhists_th228(e_simple, gamma_lines, window_sizes; binning_peak_window=binning_peak_window, e_unit=e_unit)


It makes sense to rename it, but maybe this could go along then with a PR onto the dataflow where we do a search and replace for these functions.

LisaSchlueter · 2024-12-17T16:21:06Z

Thanks for your input! I really appreciate it. I suggest the following before merging this PR:

LegendSpecFits:

rename everything th228 related into gamma
merge the two simple_calibration functions into one combined function. Same for plot recipe.
test everything on th228 and co60 data

julia-legend-dataflow:

adapt processor to new naming

metadata / legend-jldataflow-config:

add source field (see above)

fhagemann · 2024-12-20T10:28:49Z

That sounds like a great plan.
We can still define functions called simple_calibration_th228 that will just call simple_calibration_gamma, something along

simple_calibration_th228(args...; kwargs...) = simple_calibration_gamma(args...; source = :th228, kwargs...)

where the keyword argument is source, calib_type, ... or whatever.

src/chi2fit.jl

LisaSchlueter · 2025-03-07T01:21:01Z

In general, the PR looks good. Thanks @LisaSchlueter . My comments are mainly about the naming. Do we wanna stick to the isotope naming or start having more generic names?

We now have generic names peak_search_gamma(), peakhists_gamma(). There are still two options for calib_type

calib_type == :th228
calib_type == :gamma
In both cases, we apply the same methods to perform the simple calibration. The difference is that for th228, some values like the peakfinder_threshold, peakfinder_σ, peak_quantile and bin_quantile are harded-coded to good values for a LEGEND-like Th-228 spectrum.
In the case of :gamma, the user has more options to tune the simple calibration.

LisaSchlueter · 2025-03-07T01:22:28Z

@theHenks Are you happy with your request?

theHenks · 2025-03-12T11:08:10Z

@theHenks Are you happy with your request?

At a first quick look, this looks great! But I didn't find the time to look through this in detail yet. If it is not super urgently needed I would push this to after the CM.

LisaSchlueter · 2025-03-12T16:37:01Z

Yes, let's do it after the CM then. I'll probably divide this into a few smaller PRs to make it easier to track the many small changes.

LisaSchlueter added 3 commits December 12, 2024 15:23

add more generic verison of simple calibration, e.g. for co60 data

a0b628f

take most prominent peak not not largest energy

fa21052

add minimal peakshape without tail

1639f38

LisaSchlueter added the enhancement New feature or request label Dec 16, 2024

LisaSchlueter requested review from fhagemann and theHenks December 16, 2024 22:21

LisaSchlueter self-assigned this Dec 16, 2024

fhagemann reviewed Dec 16, 2024

View reviewed changes

LisaSchlueter added 2 commits December 16, 2024 15:22

update docstring and add fall-back solution for fwhm calculation

888996b

add e_unit as kwarg

421e2c3

theHenks requested changes Dec 17, 2024

View reviewed changes

LisaSchlueter added 6 commits January 10, 2025 14:04

merge commong function for th228 and generic gamma

692c1de

clean up th228 remainders and docstrings

b8dda3f

merge simple calibration plto recipes

8e924d9

add simple calibration test

13a85f9

bug fix tests

4851690

account for case when dof = 0. uncertainties can still be propagated

da82248

fhagemann reviewed Mar 7, 2025

View reviewed changes

src/chi2fit.jl Outdated Show resolved Hide resolved

LisaSchlueter and others added 2 commits March 6, 2025 16:35

add fhagemann suggestions

a84f399

Merge branch 'main' into co60

8ff7002

LisaSchlueter mentioned this pull request Mar 7, 2025

plot recipe: simple_calibration legend-exp/LegendMakie.jl#30

Open

fhagemann mentioned this pull request Mar 8, 2025

Update plot recipe for simple_calibration report legend-exp/LegendMakie.jl#31

Open

fhagemann requested a review from theHenks March 11, 2025 12:43

bug fix

db57f33

LisaSchlueter added 2 commits March 11, 2025 15:13

Merge branch 'co60' of github.com:legend-exp/LegendSpecFits.jl into co60

35f9b55

remove broadcasting for now; causes problems

cace095

LisaSchlueter marked this pull request as draft March 11, 2025 22:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generic simple_calibration #108

Generic simple_calibration #108

LisaSchlueter commented Dec 16, 2024 •

edited

Loading

fhagemann Dec 16, 2024

LisaSchlueter Dec 16, 2024 •

edited

Loading

theHenks Dec 17, 2024

LisaSchlueter Dec 17, 2024

LisaSchlueter commented Dec 16, 2024

codecov bot commented Dec 16, 2024 •

edited

Loading

theHenks left a comment

theHenks Dec 17, 2024

LisaSchlueter Dec 17, 2024

theHenks Dec 17, 2024

LisaSchlueter Mar 7, 2025

theHenks Dec 17, 2024

LisaSchlueter commented Dec 17, 2024

fhagemann commented Dec 20, 2024

LisaSchlueter commented Mar 7, 2025

LisaSchlueter commented Mar 7, 2025

theHenks commented Mar 12, 2025

LisaSchlueter commented Mar 12, 2025

Generic simple_calibration #108

Are you sure you want to change the base?

Generic simple_calibration #108

Conversation

LisaSchlueter commented Dec 16, 2024 • edited Loading

Choose a reason for hiding this comment

LisaSchlueter Dec 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LisaSchlueter commented Dec 16, 2024

codecov bot commented Dec 16, 2024 • edited Loading

Codecov Report

theHenks left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LisaSchlueter commented Dec 17, 2024

fhagemann commented Dec 20, 2024

LisaSchlueter commented Mar 7, 2025

LisaSchlueter commented Mar 7, 2025

theHenks commented Mar 12, 2025

LisaSchlueter commented Mar 12, 2025

LisaSchlueter commented Dec 16, 2024 •

edited

Loading

LisaSchlueter Dec 16, 2024 •

edited

Loading

codecov bot commented Dec 16, 2024 •

edited

Loading