Skip to content

Removal of code duplication for charge computation in trr_test_suite#244

Merged
jlenain merged 6 commits intocta-observatory:mainfrom
pcorcam:trr-charge-duplication
Dec 17, 2025
Merged

Removal of code duplication for charge computation in trr_test_suite#244
jlenain merged 6 commits intocta-observatory:mainfrom
pcorcam:trr-charge-duplication

Conversation

@pcorcam
Copy link
Contributor

@pcorcam pcorcam commented Dec 15, 2025

Removes code duplication in the trr_test_suite regarding the charge computation. More specifically:

  • ChargeContainer and ChargeComp are removed from trr_test_suite/tools_components.py;
  • The standard ChargesComponent (makers/components/charges_components.py) is now the component in the LinearityTestTool (trr_test_suite/tools_components.py);
  • The script trr_test_suite/linearity.py has been adapted accordingly - pedestals are precomputed with the standard PedestalNectarCAMCalibrationTool and its output is fed to the ChargesComponent in the LinearityTestTool.

A crosscheck was performed to test the compatibility of both charge-extraction methods. The parameters:

  • run_number = 6954
  • max_events = 500
  • window_width = 16
  • window_shift = 6
  • For standard ChargesComponent:
    • method=LocalPeakWindowSum
    • pedestal is computed with PedestalNectarCAMCalibrationTool with parameters:
      • max_events = 12000
      • events_per_slice = 5000
      • method = FullWaveFormSum
      • filter_method = None

We find good agreement between the two charge-extraction methods (see figures below); the relative difference is on average $\mu = 0.0045$% with a standard deviation $\sigma = 0.5335$%.

image image

Note that there is still quite some code duplication regarding charge computation in trr_test_suite/tools_components.py, notably in ToMComp. This will be tackled in a future PR.

@pcorcam pcorcam self-assigned this Dec 15, 2025
@pcorcam pcorcam added the enhancement New feature or request label Dec 15, 2025
@codecov
Copy link

codecov bot commented Dec 15, 2025

Codecov Report

❌ Patch coverage is 35.71429% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.00%. Comparing base (e8a4985) to head (d2f0917).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/nectarchain/trr_test_suite/tools_components.py 12.50% 7 Missing ⚠️
src/nectarchain/trr_test_suite/linearity.py 50.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #244      +/-   ##
==========================================
+ Coverage   51.71%   52.00%   +0.29%     
==========================================
  Files          80       80              
  Lines        6774     6713      -61     
==========================================
- Hits         3503     3491      -12     
+ Misses       3271     3222      -49     

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

@jlenain jlenain force-pushed the trr-charge-duplication branch from 3a1d7bb to 3753dc6 Compare December 15, 2025 16:39
@jlenain
Copy link
Collaborator

jlenain commented Dec 15, 2025

Hi @pcorcam ,

Many thanks for this ! There was a conflict with the freshly merged PR #243 . I rebased this PR against main.

@jlenain
Copy link
Collaborator

jlenain commented Dec 16, 2025

Thanks a lot also for the comparison before/after, @pcorcam, I think this is really compelling!

@jlenain jlenain self-requested a review December 16, 2025 16:38
@jlenain jlenain merged commit b49601d into cta-observatory:main Dec 17, 2025
24 of 27 checks passed
alessandromontanari pushed a commit to alessandromontanari/nectarchain that referenced this pull request Jan 27, 2026
cta-observatory#244)

* make sure the logging only happens when the container is succesfully loaded

* change to standard ChargesComponent in LinearityTestTool

* update usage of LinearityTestTool

* remove duplication of ChargeContainer and ChargeComponent

* small correction in output path for pedestal file

* Avoid using the same hardcoded value defined in several places.

---------

Co-authored-by: jlenain <jlenain@in2p3.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants