Issue 129#132
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new pixel equalization calibration workflow (and supporting CLI/pipeline wiring) and adds a spectrum-PDF facility used by the equalization likelihood fit, while refactoring gain calibration to be derived from the equalization result.
Changes:
- Added
SpectrumPDF(save/load + KDE-based interpolation) and acalibspectask/CLI command to generate PDFs from reconstructed spectra. - Added
CalibrationType.EQUALIZATIONplus a newCalibrateEqualizationlikelihood-based calibration, and refactored gain calibration to use equalization + sensor material properties. - Updated reconstruction/display/eta calibration to use an equalization matrix and expanded recon output to include an
adccolumn.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_pdf.py | New tests for SpectrumPDF (currently has pytest-signature and assertion issues). |
| src/hexsample/pdf.py | New SpectrumPDF implementation (KDE + spline, serialization, derivative, plotting). |
| src/hexsample/calibration.py | Adds equalization calibration type/metadata and implements likelihood-based equalization + refactored gain calibration. |
| src/hexsample/tasks.py | Wires equalization into reconstruct/display/eta, adds calibspec, refactors calibration tasks and filenames. |
| src/hexsample/pipeline.py | Pipeline entrypoints updated for equalization + new calibspec and split equalization vs gain calibration. |
| src/hexsample/cli.py | CLI updated: adds calibspec, adds calibgen equalization, refactors calibgen gain options. |
| src/hexsample/clustering.py | Adds adc_to_ev handling in Cluster and propagates conversion factor from calibration metadata. |
| src/hexsample/recon.py | Refactors ReconEvent to expose adc() and uses Cluster.energy() for energy calculation. |
| src/hexsample/fileio.py | Recon table schema updated to include adc column and writes event.adc() into output. |
| src/hexsample/caldb.py | Adds CalDB accessor for equalization calibration files. |
| pyproject.toml | Adds new dependencies (iminuit, joblib). |
| docs/release_notes.rst | Documents the new equalization/PDF workflow and CLI changes (contains a few typos). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+54
to
+62
| def adc(self) -> int: | ||
| """Return the total ADC count for the event. | ||
| """ | ||
| return self.cluster.pulse_height() | ||
|
|
||
| .. warning:: | ||
| This is currently using the ionization energy of Silicon to do the | ||
| conversion, assuming a detector gain of 1. We will need to do some | ||
| bookkeeping, here, to make this work reliably. | ||
| def energy(self) -> float: | ||
| """Return the energy of the event in eV. | ||
| """ | ||
| return ionization_potential * self.cluster.pulse_height() | ||
| return self.cluster.energy() |
| energy = tables.Float32Col(pos=5) | ||
| posx = tables.Float32Col(pos=6) | ||
| posy = tables.Float32Col(pos=7) | ||
| adc = tables.Int32Col(pos=5) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.