search

cta-observatory / nectarchain

NectarCAM high level analysis tools
https://nectarchain.readthedocs.io
BSD 3-Clause "New" or "Revised" License
7 stars 18 forks source link

Add container/component/tool for pedestal estimation #115

Closed tibaldo closed 7 months ago

tibaldo commented 7 months ago

This PR add a tool/component dedicated to estimate pedestals based on data that contain events of type SKYPEDESTAL. Pedestals are stored in a dedicated container following the nectarchain conventions (pixels_id etc.). Several methods for filtering the data are available based on timestamps of the events, standard deviations of the waveforms and the charge distribution. In order to reduce the memory load the tool is built in such a way that pedestals are estimated in slices with a number of events events_per_slice and then results combined at the end. PS: I'm not sure why in the file comparison I see the renaming master-> main which is already integrated in the upstream repository

jlenain commented 7 months ago

CI tests are currently failling due to #116 . #117 should fix it.

codecov[bot] commented 7 months ago

Codecov Report

Attention: Patch coverage is 92.07650% with 29 lines in your changes are missing coverage. Please review.

Project coverage is 39.43%. Comparing base (51b976c) to head (1f09a0d).

Files Patch % Lines
src/nectarchain/makers/core.py 26.66% 11 Missing :warning:
...c/nectarchain/makers/calibration/pedestalMakers.py 85.93% 9 Missing :warning:
.../nectarchain/makers/component/PedestalComponent.py 93.38% 9 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #115 +/- ## =========================================== + Coverage 28.54% 39.43% +10.88% =========================================== Files 62 65 +3 Lines 4102 4453 +351 =========================================== + Hits 1171 1756 +585 + Misses 2931 2697 -234 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

jlenain commented 7 months ago

PS: I'm not sure why in the file comparison I see the renaming master-> main which is already integrated in the upstream repository

I think it is, maybe, because you pull from the main branch of the repo into your dev branch before pushing your commits into it ? If so, new features introduced in the main are duplicated in your dev branch. This should not be any issue at merge (we'll clean the commit log), but the proper workflow is to ignore any new commit from the main in your dev branch. Any potential conflicts with the main will be flagged, and can be fixed, when we merge the PR.