Add uncertainty column to effective area and fix PSF_TABLE column names by orelgueta · Pull Request #32 · Eventdisplay/Converters

orelgueta · 2026-05-11T16:46:54Z

Add optional include_uncertainty parameter to write_histo2D function
Enable uncertainty column (EFFAREA_ERR) for effective area table
Fix PSF_TABLE column names: MIGRA->RAD and swap THETA/RAD positions for GADF compliance
Uncomment write_psf_table call to enable 3D PSF table generation

- Add optional include_uncertainty parameter to write_histo2D function - Enable uncertainty column (EFFAREA_ERR) for effective area table - Fix PSF_TABLE column names: MIGRA->RAD and swap THETA/RAD positions for GADF compliance - Uncomment write_psf_table call to enable 3D PSF table generation

Copilot

Pull request overview

This PR updates DL3 IRF FITS generation to better align with GADF expectations by adding an optional uncertainty column for 2D histogram tables (enabled for effective area) and by adjusting PSF_TABLE column naming/axis semantics, while also enabling PSF_TABLE output in the conversion tool.

Changes:

Add include_uncertainty option to write_histo2D() and enable it for the effective area HDU (writes EFFAREA_ERR).
Update PSF_TABLE column names (MIGRA_* → RAD_*) and adjust THETA/RAD ordering intent.
Enable PSF_TABLE generation in convertSensitivityFilesToFITS.cpp by uncommenting the call.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
DL3-IRFs/src/VDL3IRFs.cpp	Adds optional uncertainty column handling in `write_histo2D()`, enables it for AEFF, and updates PSF_TABLE column names/comments.
DL3-IRFs/src/convertSensitivityFilesToFITS.cpp	Enables writing the PSF_TABLE HDU during conversion.
DL3-IRFs/inc/VDL3IRFs.h	Extends `write_histo2D()` signature with `include_uncertainty` default parameter.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Set HDUCLAS2=PSF for PSF_TABLE type in write_fits_table_header(). Previously only PSF_3GAUSS was handled, causing PSF_TABLE to miss required GADF classification keywords.

…onverters into clean-uncertainty-psf-fix

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

DL3-IRFs/src/VDL3IRFs.cpp:388

The comment // mig is misleading in write_psf_table() (this is PSF content, not a migration axis). Please rename it to reflect what the array column actually contains (e.g. RPSF / flattened 3D PSF values) to avoid confusion when maintaining the axis ordering.

   // Field of view offset angle (z-axis, THETA)
   char z_form[10];
   sprintf( z_form, "%dE", h->GetNbinsZ() );
   // mig
   char m_form[10];

Change PSF HDU names to match their HDUCLAS4 values: - write_psf_table: 'POINT SPREAD FUNCTION' -> 'PSF_TABLE' - write_psf_gauss: 'POINT SPREAD FUNCTION' -> 'PSF_3GAUSS' This prevents duplicate EXTNAME values and makes HDU lookups unambiguous.

Replace fixed-size char buffer with std::string for err_col_name: - Eliminates potential truncation risk from snprintf - No need to check snprintf return value - Fix comment: 'static buffer' -> 'using std::string' - More idiomatic C++ code

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Address reviewer feedback: - Revert PSF_3GAUSS EXTNAME to 'POINT SPREAD FUNCTION' for backward compatibility with existing tools (e.g., test_cta_file.py) - Keep PSF_TABLE EXTNAME as 'PSF_TABLE' for the 3D table - Update HDUCLAS2 condition to recognize 'POINT SPREAD FUNCTION' - Add null pointer check for AngularPSF2DEtrue_offaxis histogram - Check write_psf_table() return value and log errors - Skip PSF_TABLE writing with warning if histogram not found This maintains compatibility with downstream consumers while still providing unique EXTNAME values for the two different PSF formats.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Fix GADF compliance issue where HDUCLAS4 was set to 'POINT SPREAD FUNCTION' instead of 'PSF_3GAUSS': - write_psf_gauss() now passes 'PSF_3GAUSS' to write_fits_table_header() - Update write_fits_table_header() condition to recognize 'PSF_3GAUSS' - EXTNAME remains 'POINT SPREAD FUNCTION' (backward compatible) - HDUCLAS4 is now 'PSF_3GAUSS' (GADF/ogadf_schema compliant) This properly separates the display name (EXTNAME) from the IRF class identifier (HDUCLAS4) used for validation.

Fix error handling so write_psf_table() and write_psf_gauss() properly report write failures: - Capture write_table() return value (write_psf_table was ignoring it) - Only write FITS headers if table write succeeded - Return actual success/failure status instead of always returning true This allows the error check in convertSensitivityFilesToFITS.cpp to actually detect and report PSF_TABLE write failures.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Prevent write_fits_table_header() from being called on write failures, which could corrupt previously-open HDUs. Applied to: - write_edisp(): EDISP_2D header - write_background_3D_from_2d(): BKG_3D header - write_background(): BKG_2D header - write_effarea(): AEFF_2D header - write_diffsens(): DIFFSENS_2D header This matches the pattern already applied to write_psf_gauss() and write_psf_table() and ensures FITS file integrity when writes fail.

…onverters into clean-uncertainty-psf-fix

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

+   // Column names - create error column name using std::string
+   string err_col_name;
+   if( include_uncertainty )
+   {
+       err_col_name = string(col_name) + "_ERR";
+   }
+
+   // Arrays sized for maximum columns; fits_create_tbl uses only first nCol entries
+   char* tType[maxCol] = { (char*)"ENERG_LO",
+                           (char*)"ENERG_HI",
+                           (char*)"THETA_LO",
+                           (char*)"THETA_HI",
+                           (char*)col_name,
+                           include_uncertainty ? (char*)err_col_name.c_str() : (char*)"" };
+


orelgueta requested a review from Copilot May 11, 2026 16:47

Copilot started reviewing on behalf of orelgueta May 11, 2026 16:47 View session

Copilot AI reviewed May 11, 2026

View reviewed changes

Comment thread DL3-IRFs/src/convertSensitivityFilesToFITS.cpp Outdated

Comment thread DL3-IRFs/src/VDL3IRFs.cpp Outdated

orelgueta and others added 3 commits May 11, 2026 18:55

Fix comment about asymetric errors

d11d1a5

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Fix PSF_TABLE GADF classification keywords

93e5207

Set HDUCLAS2=PSF for PSF_TABLE type in write_fits_table_header(). Previously only PSF_3GAUSS was handled, causing PSF_TABLE to miss required GADF classification keywords.

Merge branch 'clean-uncertainty-psf-fix' of github.com:Eventdisplay/C…

109be51

…onverters into clean-uncertainty-psf-fix

orelgueta requested a review from Copilot May 11, 2026 18:33

Copilot started reviewing on behalf of orelgueta May 11, 2026 18:34 View session

Copilot AI reviewed May 11, 2026

View reviewed changes

Comment thread DL3-IRFs/src/convertSensitivityFilesToFITS.cpp Outdated

Comment thread DL3-IRFs/src/VDL3IRFs.cpp Outdated

orelgueta added 2 commits May 11, 2026 20:43

orelgueta requested a review from Copilot May 11, 2026 18:51

Copilot started reviewing on behalf of orelgueta May 11, 2026 18:51 View session

Copilot AI reviewed May 11, 2026

View reviewed changes

Comment thread DL3-IRFs/src/VDL3IRFs.cpp

Comment thread DL3-IRFs/src/VDL3IRFs.cpp

Comment thread DL3-IRFs/src/convertSensitivityFilesToFITS.cpp Outdated

orelgueta requested a review from Copilot May 12, 2026 07:46

Copilot started reviewing on behalf of orelgueta May 12, 2026 07:47 View session

Copilot AI reviewed May 12, 2026

View reviewed changes

Comment thread DL3-IRFs/src/VDL3IRFs.cpp Outdated

Comment thread DL3-IRFs/src/convertSensitivityFilesToFITS.cpp

orelgueta added 2 commits May 12, 2026 10:22

orelgueta requested a review from Copilot May 12, 2026 11:18

Copilot started reviewing on behalf of orelgueta May 12, 2026 11:18 View session

Copilot AI reviewed May 12, 2026

View reviewed changes

Comment thread DL3-IRFs/src/VDL3IRFs.cpp Outdated

Comment thread DL3-IRFs/src/convertSensitivityFilesToFITS.cpp Outdated

orelgueta and others added 3 commits May 12, 2026 13:28

Change brackets

5d2508e

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Merge branch 'clean-uncertainty-psf-fix' of github.com:Eventdisplay/C…

f163e07

…onverters into clean-uncertainty-psf-fix

orelgueta requested a review from Copilot May 12, 2026 11:41

Copilot started reviewing on behalf of orelgueta May 12, 2026 11:42 View session

Copilot AI reviewed May 12, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add uncertainty column to effective area and fix PSF_TABLE column names#32

Add uncertainty column to effective area and fix PSF_TABLE column names#32
orelgueta wants to merge 12 commits into
mainfrom
clean-uncertainty-psf-fix

orelgueta commented May 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

orelgueta commented May 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants