TRR Analysis script updates#239
Conversation
jlenain
left a comment
There was a problem hiding this comment.
Thanks a lot, @medha1996c.
However, major changes are needed before this PR could be accepted: it re-implements a lot of classes already existing within nectarchain. Please let's avoid code duplication, and try to use the already existing implementations.
src/nectarchain/user_scripts/mchakraborty/trr_codes/.nfs00000001e3b175b200000b8e
Outdated
Show resolved
Hide resolved
src/nectarchain/user_scripts/mchakraborty/trr_codes/.nfs00000001e484635400000b8d
Outdated
Show resolved
Hide resolved
src/nectarchain/user_scripts/mchakraborty/trr_codes/.nfs00000001e62200d8000009ae
Outdated
Show resolved
Hide resolved
src/nectarchain/user_scripts/mchakraborty/trr_codes/ChargeResolution.py
Outdated
Show resolved
Hide resolved
src/nectarchain/user_scripts/mchakraborty/trr_codes/tools_components.py
Outdated
Show resolved
Hide resolved
src/nectarchain/user_scripts/mchakraborty/trr_codes/tools_components.py
Outdated
Show resolved
Hide resolved
src/nectarchain/user_scripts/mchakraborty/trr_codes/tools_components.py
Outdated
Show resolved
Hide resolved
src/nectarchain/user_scripts/mchakraborty/trr_codes/tools_components.py
Outdated
Show resolved
Hide resolved
src/nectarchain/user_scripts/mchakraborty/trr_codes/tools_components.py
Outdated
Show resolved
Hide resolved
src/nectarchain/user_scripts/mchakraborty/trr_codes/tools_components.py
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #239 +/- ##
==========================================
- Coverage 51.90% 50.47% -1.44%
==========================================
Files 82 83 +1
Lines 6845 7067 +222
==========================================
+ Hits 3553 3567 +14
- Misses 3292 3500 +208 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
src/nectarchain/user_scripts/mchakraborty/trr_codes/tools_components.py
Outdated
Show resolved
Hide resolved
|
Hi @medha1996c ! |
|
Hi @medha1996c , The last push from yesterday mixes several PRs, I suspect this is due to a misbehaved rebase, is that correct? Please undo the last changes, rebase properly your branch onto main, and push again. |
Hi @medha1996c , Thanks for the last commits. Coming back to this: would it be possible please to implement some unit tests, especially for the |
| "default None, ff_coefficients=1" | ||
| "1- Independent, 2- 2D Gaussian model by Anastasiia, else ff_coefficients=1", | ||
| required=False, | ||
| default=None, |
There was a problem hiding this comment.
There is an inconsistency here: the model for the flat-field is declared as int, though taking None as default value.
When testing the integration of this test within the TRR GUI, this (correctly) fails with the following error:
/home/jlenain/local/src/python/cta-observatory/ctapipe_io_nectarcam/src/ctapipe_io_nectarcam/__init__.py:42: UserWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html. The pkg_resources package is slated for removal as early as 2025-11-30. Refrain from using this package or pin to Setuptools<81.
from pkg_resources import resource_filename
usage: charge_resolution.py [-h] [-r RUN_FILE] [-e EVTS] [-o OUTPUT]
[-t TEMPERATURE] [-ff FF_MODEL]
charge_resolution.py: error: argument -ff/--ff_model: invalid int value: 'None'This reverts commit 94d83c3.
… with several run lists, for different tests for the TRR, in the `resources` sub-folder).
Scripts for ChargeResolution using FF + NSB runs are added. Module added to reject bad_pixels, use per pixel per temperature gain (from SPE results by Halim) and FF coefficients using Anastasiia's script output.