Skip to content

Conversation

@pcorcam
Copy link
Contributor

@pcorcam pcorcam commented Aug 19, 2025

First attempt at creating a Category A calibration file, inspired by lstcam_calib.tools.create_calibration_file. For now, this CalibrationWriterNectarCAM tool works as follows:

  1. Take the .h5 output files from a NectarCAMCalibrationTool, which contain results of pedestal, etc., in a NectarCAMPedestalContainer, etc.
  2. Copy the results in relevant ctapipe containers that are used to produce the Category A calibration file. In some cases, this requires some further computations. For example, in nectarchain, the NectarCAMPedestalContainer contains the average pedestal per pixel per sample, while in ctapipe, the PedestalContainer expects the pedestal per pixel averaged over all samples.
  3. Write the Category A calibration file in either .h5, .fits, or .fits.gz formats, as done in lstcam_calib.

An usage example of this tool has been added to user_scripts/pcorrea/example_create_catA_calibration_file.py.

I chose this way of implementing this tool so I don't have to make any changes to the already existing nectarchain tools at this stage. I also opted to not depend on lstcam_calib in nectarchain here, since lstcam_calib uses their custom EventSource in their CalibrationWriter tool, which also performs the actual computation of the calibration coefficients for a specific run.

However, this version of the tool is a bit cumbersome since you just copy information from NectarCAMContainers to standard ctapipe containers. Probably there is a better way to deal with this, but this makes more sense to implement after creating a pipeline that enchains all the NectarCAMCalibrationTools.

@tibaldo
Copy link
Member

tibaldo commented Aug 25, 2025

Hi @pcorcam, thank you for doing this! I'm a bit confused about the pedestal subtraction. I think that for the NectarCAM we want a correction per sample and not averaged over all samples for each pixel?

@pcorcam
Copy link
Contributor Author

pcorcam commented Aug 25, 2025

Hi @tibaldo! The reason why I averaged over the samples is because, in first instance, I wanted to create a Category-A calibration file à la lstcam_calib, just to make sure it is (or should in principle be) compatible with the current EVB. But indeed, it is something I would like to discuss, i.e. whether we can add a per-sample calibration at the EVB level. Hope this clarifies!

@tibaldo
Copy link
Member

tibaldo commented Aug 25, 2025

Hi @tibaldo! The reason why I averaged over the samples is because, in first instance, I wanted to create a Category-A calibration file à la lstcam_calib, just to make sure it is (or should in principle be) compatible with the current EVB. But indeed, it is something I would like to discuss, i.e. whether we can add a per-sample calibration at the EVB level. Hope this clarifies!

Hi @pcorcam okay let's discuss it at the first occasion also with the Event builder experts.

@codecov
Copy link

codecov bot commented Sep 3, 2025

Codecov Report

❌ Patch coverage is 1.63265% with 241 lines in your changes missing coverage. Please review.
✅ Project coverage is 51.38%. Comparing base (d5724a6) to head (8cdb764).
⚠️ Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
.../nectarchain/tools/create_catA_calibration_file.py 0.00% 203 Missing ⚠️
src/nectarchain/utils/metadata.py 0.00% 38 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #207      +/-   ##
==========================================
- Coverage   52.25%   51.38%   -0.87%     
==========================================
  Files          76       78       +2     
  Lines        6329     6475     +146     
==========================================
+ Hits         3307     3327      +20     
- Misses       3022     3148     +126     

☔ 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
Copy link
Collaborator

jlenain commented Sep 15, 2025

Many thanks for this, @pcorcam !
Following our discussions during the Scientific Software meeting, shall we wait for the functional test (https://redmine.cta-observatory.org/issues/54306) to be complete before merging this PR, or should we do it right away ?

@pcorcam
Copy link
Contributor Author

pcorcam commented Sep 15, 2025

I think we can wait a bit; that way, if there is an issue arising from the functional test, we can update it in this pull request. And there is no rush to add it anyways, since we will be developing other parts of the code first!

@jlenain
Copy link
Collaborator

jlenain commented Sep 29, 2025

Since the functional test documented at https://redmine.cta-observatory.org/issues/54306 seems successful, let's merge this PR !

Many thanks !

@jlenain jlenain self-requested a review September 29, 2025 14:02
@jlenain jlenain merged commit 412b951 into cta-observatory:main Sep 29, 2025
8 of 10 checks passed
tibaldo pushed a commit to tibaldo/nectarchain that referenced this pull request Oct 2, 2025
…libration files (cta-observatory#207)

* Add script to obtain metadata of `ctapipe` and `nectarchain`. Taken from `lstcam_calib` with minor adaptations for `nectarchain`.

* added group name as option to HDF5 readout, particularly useful for pedestal output files that use various groups to store data

* first attempt for a Tool that writes a category A output file from NectarCAM containers

* Add example script for the usage of the CalibrationWriterNectarCAM tool

* ensure that filled arrays have the correct type (based on `lstcam_calib` example file)

* do one-padding instead of zero-padding to expand arrays related to pixel status with missing pixels (so a missing pixel = True for those that are added, i.e. due that were not included due to hardware failure)

* fixed hardware failing pixel status bug
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants