Change request #2670

Support ENERG colname for energy dispersion fits file

Added by Specovius Andreas over 6 years ago. Updated over 6 years ago.

Status:ClosedStart date:09/04/2018
Priority:NormalDue date:
Assigned To:Knödlseder Jürgen% Done:

100%

Category:-
Target version:1.6.0
Duration:

Description

Right now, GCTAEdisp2D initializes its energy axis with the hardcoded name “ETRUE”:
m_inx_etrue = m_edisp.axis("ETRUE")

However, specifications for gamma-ray data formats suggest “ENERG”:
https://gamma-astro-data-formats.readthedocs.io/en/latest/irfs/full_enclosure/edisp/index.html#edisp-2d

Maybe this class should be changed to support multiple column names.

Recurrence

No recurrence.

History

#1 Updated by Knödlseder Jürgen over 6 years ago

  • Status changed from New to Pull request
  • Assigned To set to Knödlseder Jürgen
  • Target version set to 1.6.0
  • % Done changed from 0 to 100

I recall that we had at some point a discussion about names of energy columns and wanted to have a distinction between true and reconstructed energies in the column names, but I could not find any trace of that discussion.

Anyway, I changed the code so that both ETRUE and ENERG column names are supported. Code is merged into devel (you may check if everything is okay since I have not made an explicit code check).

#2 Updated by Knödlseder Jürgen over 6 years ago

  • Status changed from Pull request to Closed

I also changed the column name in csroot2caldb so that newly generated IRFs now have ENERG as column name.

Code is merged into devel.

#3 Updated by Specovius Andreas over 6 years ago

Knödlseder Jürgen wrote:

I recall that we had at some point a discussion about names of energy columns and wanted to have a distinction between true and reconstructed energies in the column names, but I could not find any trace of that discussion.

I see your point and aggree with you. I don’t know, maybe the suggested data format specs are deprecated?

#4 Updated by Specovius Andreas over 6 years ago

Knödlseder Jürgen wrote:

Anyway, I changed the code so that both ETRUE and ENERG column names are supported. Code is merged into devel (you may check if everything is okay since I have not made an explicit code check).

Unfortunately in the code you search for the table ENERG which is an axis. An exception is raised. Line
1339> if (m_edisp.has_table("ENERG")) {

should be replaced with
1339> if (m_edisp.has_axis("ENERG")) {

#5 Updated by Knödlseder Jürgen over 6 years ago

  • Status changed from Closed to Feedback

Sorry for that, is corrected now. Can you check again?

#6 Updated by Specovius Andreas over 6 years ago

Knödlseder Jürgen wrote:

Sorry for that, is corrected now. Can you check again?

The updated implementation works correctly

#7 Updated by Knödlseder Jürgen over 6 years ago

  • Status changed from Feedback to Closed

Great, thanks.

Also available in: Atom PDF