add a method to extract directly the waveform from PSL by tdixon97 · Pull Request #164 · legend-exp/reboost

tdixon97 · 2026-02-05T23:33:23Z

@SalehGiovanna this would allow you to emulate a full (ideal) waveform in python.

This is rather slow and probably not something we will use for post-processing but it might be good for tuning the models.

This is a few orders of magnitude faster than SSD directly but still quite alot slower than maximum_current. This is because it computes the full waveform not only searches the maximum.

Possible enhancements are to consider the phi-dependence, interpolate templates between time-steps etc

codecov · 2026-02-05T23:39:40Z

Codecov Report

❌ Patch coverage is 60.63830% with 37 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.63%. Comparing base (409f161) to head (df3ff51).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/reboost/hpge/psd.py	38.00%	31 Missing ⚠️
src/reboost/hpge/utils.py	86.36%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #164      +/-   ##
==========================================
+ Coverage   68.98%   69.63%   +0.64%     
==========================================
  Files          33       33              
  Lines        2631     2720      +89     
==========================================
+ Hits         1815     1894      +79     
- Misses        816      826      +10

☔ 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.

Copilot

Pull request overview

This pull request adds a new method to extract full waveforms directly from a pulse shape library (PSL) for HPGe detectors. The implementation enables emulating ideal waveforms in Python for model tuning purposes, though it is slower than the existing maximum_current function since it computes the full waveform rather than just finding the maximum.

Changes:

Added waveform_from_pulse_shape_library() function and supporting _get_psl_waveforms_impl() to extract full waveforms from pulse shape libraries
Made the t0 field optional in get_hpge_pulse_shape_library() to support pulse shape libraries without explicit t0 values
Added test coverage for the new waveform extraction functionality

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File	Description
src/reboost/hpge/psd.py	Implements new functions to extract full waveforms from pulse shape library by summing energy-weighted templates
src/reboost/hpge/utils.py	Makes t0 field optional in pulse shape library loading with default value of 0 and adds default empty units for waveform field
tests/hpge/test_waveforms.py	Adds basic test to verify waveform extraction produces correct summed values

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

src/reboost/hpge/utils.py

src/reboost/hpge/psd.py

tests/hpge/test_waveforms.py

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

docs/source/manual/psd.md

tests/hpge/test_waveforms.py

src/reboost/hpge/utils.py

src/reboost/hpge/psd.py

docs/source/manual/psd.md

tdixon97 · 2026-02-06T11:34:11Z

@copilot in the pulse shape libraries we typically store two waveforms on two angles 0 and 45 deg. Typically there is a repeating symmetry every 45 deg.
I would like to add an option to pass the phi for every hit to reboost and then it choses the PSL that is closer.

Copilot · 2026-02-06T11:34:20Z

@tdixon97 I've opened a new pull request, #165, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Co-authored-by: tdixon97 <56904179+tdixon97@users.noreply.github.com>

…D array Co-authored-by: tdixon97 <56904179+tdixon97@users.noreply.github.com>

…0_deg) Co-authored-by: tdixon97 <56904179+tdixon97@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-12T00:21:43Z

src/reboost/hpge/psd.py

+            if has_phi and phi_t is not None:
+                phi_idx = _get_phi_idx(phi_t[j], phi_grid)


When the pulse shape library is 4D (r,z,phi,time) but phi is not provided, this falls through to the else branch and indexes pulse_shape_library[r_idx, z_idx] (shape (phi,time)), which cannot be added into waveforms[i] (shape (time,)) and will raise a broadcasting error. If the intended behavior is to default to the first phi template when phi is omitted (as the tests/docstring suggest), handle has_phi with phi is None by using phi_idx=0 (or explicitly selecting a default phi) instead of the 3D indexing path.

Suggested change

if has_phi and phi_t is not None:

phi_idx = _get_phi_idx(phi_t[j], phi_grid)

if has_phi:

# If phi_t is not provided but the pulse shape library has a phi

# dimension, default to the first phi template (index 0).

if phi_t is not None:

phi_idx = _get_phi_idx(phi_t[j], phi_grid)

else:

phi_idx = 0

Copilot · 2026-02-12T00:21:43Z

src/reboost/hpge/utils.py

+    else:
+        t0 = 0
+        t0_u = "ns"

-    t0_u = data["t0"].attrs["units"]
+    dt = data["dt"].value
    dt_u = data["dt"].attrs["units"]


t0 is optional now, but when it is missing you hardcode t0_u = "ns" and later set tu = t0_u. If the file’s dt units are not ns (e.g. us), times = t0 + ... * dt will be numerically correct but the returned t_units (and implied units for times) will be wrong. Consider setting t0_u = dt_u (or tu = dt_u) when t0 is absent, and keep the unit-consistency check strict.

Copilot · 2026-02-12T00:21:44Z

src/reboost/hpge/utils.py

+            if "units" not in data[phi_field].attrs:
+                data[phi_field].attrs["units"] = ""
+            wf = np.nan_to_num(
+                data[phi_field].view_as("np", with_units=True), nan=out_of_bounds_val
+            )
+            waveform_list.append(wf.m)
+
+        # Stack into 4D array (r, z, phi, time)
+        waveforms_4d = np.stack(waveform_list, axis=2)
+        phi_array = np.array(phi_angles)
+    else:
+        # No phi-dependent fields, use the base field
+        if "units" not in data[field].attrs:
+            data[field].attrs["units"] = ""
+        wf = np.nan_to_num(data[field].view_as("np", with_units=True), nan=out_of_bounds_val)
+        waveforms_4d = wf.m


This function mutates the on-disk LGDO object (data[...].attrs["units"] = "") as a fallback when the units attribute is missing. That side effect can be surprising and may not be supported for all LGDO backends. Prefer handling missing units without modifying data (e.g., read without units and treat as dimensionless, or use a local default when interpreting the array).

Suggested change

if "units" not in data[phi_field].attrs:

data[phi_field].attrs["units"] = ""

wf = np.nan_to_num(

data[phi_field].view_as("np", with_units=True), nan=out_of_bounds_val

)

waveform_list.append(wf.m)

# Stack into 4D array (r, z, phi, time)

waveforms_4d = np.stack(waveform_list, axis=2)

phi_array = np.array(phi_angles)

else:

# No phi-dependent fields, use the base field

if "units" not in data[field].attrs:

data[field].attrs["units"] = ""

wf = np.nan_to_num(data[field].view_as("np", with_units=True), nan=out_of_bounds_val)

waveforms_4d = wf.m

if "units" in data[phi_field].attrs:

# Field has units: read as quantity and use magnitudes

wf_q = np.nan_to_num(

data[phi_field].view_as("np", with_units=True),

nan=out_of_bounds_val,

)

waveform_list.append(wf_q.m)

else:

# Field has no units: read as plain numpy array and treat as dimensionless

wf_arr = np.nan_to_num(

data[phi_field].view_as("np", with_units=False),

nan=out_of_bounds_val,

)

waveform_list.append(wf_arr)

# Stack into 4D array (r, z, phi, time)

waveforms_4d = np.stack(waveform_list, axis=2)

phi_array = np.array(phi_angles)

else:

# No phi-dependent fields, use the base field

if "units" in data[field].attrs:

wf_q = np.nan_to_num(

data[field].view_as("np", with_units=True),

nan=out_of_bounds_val,

)

waveforms_4d = wf_q.m

else:

wf_arr = np.nan_to_num(

data[field].view_as("np", with_units=False),

nan=out_of_bounds_val,

)

waveforms_4d = wf_arr

Copilot · 2026-02-12T00:21:44Z

tests/hpge/test_waveforms.py

+    waveforms = waveform_from_pulse_shape_library(edep, r, z, pulse_shape_library_with_phi)
+
+    assert waveforms.to_numpy().shape == (1, 100)


This test claims that omitting phi on a phi-enabled library defaults to the first phi template, but it only asserts the output shape. Add an assertion on the waveform values (e.g., that the result matches the 0° template scaling) so the intended behavior is actually verified.

Copilot · 2026-02-12T00:21:44Z

src/reboost/hpge/psd.py

+        phi_grid=pulse_shape_library.phi,
+    )
+
+    return units.attach_units(ak.Array(waveforms), "keV")


This function returns a "current waveform" but attaches units of "keV" unconditionally. Since the template/library waveform amplitude is generally unitless/arbitrary (and maximum_current explicitly returns unitless current), attaching energy units here is misleading and can cause downstream unit-conversion confusion. Consider returning an array without units (or, if waveform units are meaningful, store and propagate the waveform units from the library).

Suggested change

return units.attach_units(ak.Array(waveforms), "keV")

return ak.Array(waveforms)

tdixon97 force-pushed the waveforms branch from dba6575 to ea0dca1 Compare February 5, 2026 23:34

tdixon97 requested a review from Copilot February 5, 2026 23:50

Copilot started reviewing on behalf of tdixon97 February 5, 2026 23:51 View session

Copilot AI reviewed Feb 5, 2026

View reviewed changes

tdixon97 requested a review from Copilot February 6, 2026 11:20

Copilot started reviewing on behalf of tdixon97 February 6, 2026 11:21 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

Copilot AI mentioned this pull request Feb 6, 2026

Add phi-angle support to pulse shape library waveform extraction #165

Merged

tdixon97 and others added 7 commits February 12, 2026 00:10

add a method to extract directly the waveform from PSL

fb3c140

style: pre-commit fixes

15f3463

[docs] add documentation

58652a4

[docs] add documentation

045d79e

style: pre-commit fixes

8c92fbc

[docs] improve docs

7de726f

[tests] improve tests and catching edge cases

fd28734

tdixon97 force-pushed the waveforms branch from 31f0e82 to fd28734 Compare February 12, 2026 00:10

tdixon97 and others added 10 commits February 12, 2026 00:14

Update docs/source/manual/psd.md

ae61fc7

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update docs/source/manual/psd.md

8817729

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update src/reboost/hpge/psd.py

42d039d

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update docs/source/manual/psd.md

df2546d

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Add phi-angle support to waveform extraction from PSL

b092e5d

Co-authored-by: tdixon97 <56904179+tdixon97@users.noreply.github.com>

Address code review feedback: use np.inf and improve clarity

18b088a

Co-authored-by: tdixon97 <56904179+tdixon97@users.noreply.github.com>

Simplify phi-angle documentation section

33c4e83

Co-authored-by: tdixon97 <56904179+tdixon97@users.noreply.github.com>

Change PSL loading to read multiple phi-dependent fields instead of 4…

5cef386

…D array Co-authored-by: tdixon97 <56904179+tdixon97@users.noreply.github.com>

Use zero-padded 3-digit phi angles in field names (e.g., waveforms_00…

29cb320

…0_deg) Co-authored-by: tdixon97 <56904179+tdixon97@users.noreply.github.com>

style fix

df3ff51

tdixon97 requested a review from Copilot February 12, 2026 00:15

Copilot started reviewing on behalf of tdixon97 February 12, 2026 00:16 View session

Copilot AI reviewed Feb 12, 2026

View reviewed changes

		if has_phi and phi_t is not None:
		phi_idx = _get_phi_idx(phi_t[j], phi_grid)

-            if has_phi and phi_t is not None:
-                phi_idx = _get_phi_idx(phi_t[j], phi_grid)
+            if has_phi:
+                # If phi_t is not provided but the pulse shape library has a phi
+                # dimension, default to the first phi template (index 0).
+                if phi_t is not None:
+                    phi_idx = _get_phi_idx(phi_t[j], phi_grid)
+                else:
+                    phi_idx = 0

		waveforms = waveform_from_pulse_shape_library(edep, r, z, pulse_shape_library_with_phi)

		assert waveforms.to_numpy().shape == (1, 100)

	return units.attach_units(ak.Array(waveforms), "keV")
	return ak.Array(waveforms)

Conversation

tdixon97 commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tdixon97 commented Feb 6, 2026

Uh oh!

Copilot AI commented Feb 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tdixon97 commented Feb 5, 2026 •

edited

Loading

codecov bot commented Feb 5, 2026 •

edited

Loading