Consistent column names within IRFs about gamma-astro-data-formats

[cdeil]

@TarekHC - Thanks for having a detailed look.

Personally I don't care so much about consistent reco / true energy axis names. As you say, in the event list ENERGY is reco energy and no-one is in favour of changing this (so far), so full consistency where all reco energy quantities have RECO in their name is out the door anyways.

That said, energy axis names should be as simple and consistent throughout, so for sure pull requests to change energy axis names for more consistency or correctness, or alternatively extra comments clarifying which energy axis is reco or true energy are welcome.

For background, as you suspect, it's a reco energy axis. Background is computed and applied directly in reco energy, there is no EDISP associated with the background models. So +1 to clarify this here. As I said, I don't care or follow the discussions on energy axis column names much.
Maybe you can make a PR with a suggestion and then we review / decide on it tomorrow in the telcon?

[TarekHC]

Hi @cdeil ,

For background, as you suspect, it's a reco energy axis. Background is computed and applied directly in reco energy, there is no EDISP associated with the background models. So +1 to clarify this here. As I said, I don't care or follow the discussions on energy axis column names much.

Yes, a clarification would be enough. It would be more consistent (at least within the IRFs) to use ERECO, but I don't care that much either.

Maybe you can make a PR with a suggestion and then we review / decide on it tomorrow in the telcon?

I doubt I'll find the time to do it before tomorrow... But I will do it this week, hopefully. For now, specifying the energy we refer to in all the "ENERG" column names will be enough, with a slightly more detailed explanation in the background.

Cheers!

[cdeil]

In #79 (comment) @TarekHC comments:

I also added "True" or "Reconstructed" to all energy axes (following discussion in #63 ). The labelling is still terribly inconsistent (background ENERGY is reconstructed, while in the effective area is ENERGY refers to true energy and ERECO to the reconstructed one... a mess!).

I think we should just get rid of "ERECO" as an axis name and it'll be better.
The distinction which axis it is can be made via header keywords and docs.

@TarekHC - do you remember how it got introduced? I think it was you or me when we added the case of the AEFF with reco energy axis?
In any case what we have now (inconsistent naming) is bad.

As far as I can see, science tools don't support any IRF with an "ERECO" axis yet, or it would show up here:

• https://github.com/gammapy/gammapy/search?utf8=%E2%9C%93&q=ERECO&type=
• https://github.com/gammalib/gammalib/search?utf8=%E2%9C%93&q=ERECO&type=

So let's get #79 in first, and then tackle this.
Note that the description of IRF axis names still doesn't mention ERECO, so a PR is needed in any case:
http://www.gae.ucm.es/~thassan/gamma/irfs/irf_axes/index.html

[TarekHC]

@TarekHC - do you remember how it got introduced? I think it was you or me when we added the case of the AEFF with reco energy axis?

Yes, I think I did the PR, but the fact of including AEFF vs ERECO was supported by you and Gernot. I was actually supporting the fact of not including/supporting AEFF vs ERECO in the specs.

I can simply remove AEFF vs ERECO from the full-enclosure EFFAREA format page in #79 if you want.

[cdeil]

I can simply remove AEFF vs ERECO from the full-enclosure EFFAREA format page in #79 if you want.

Maybe let's try to get #79 in and keep that discussion separate.
#79 is already large / long and is about HDUCLASS and point-like.

[TarekHC]

Maybe let's try to get #79 in and keep that discussion separate.

Ok, agreed.

[cdeil]

The AEFF_RECO response as combined AEFF / EDISP is being used, so I'm -1 to drop it from the spec.

But this whole section describing it:
http://gamma-astro-data-formats.readthedocs.io/en/latest/irfs/full_enclosure/effective_area/index.html#effective-area-vs-reconstructed-energy
could just be reduced to a few sentences describing what it is and pointing out

HDUCLAS4 = ‘AEFF_2D_RECO’

as the only difference, and the difference in column names can just be dropped (ERECO -> ENERGY). This is easier for readers / writers to implement and using a header key to distinguish the two cases is just as clear.

[cdeil]

@TarekHC - With #85 merged, is this fully addressed? Or is there something else that should be changed or explanations made clearer? (please make a suggestion or PR for a change or close this issue)

[TarekHC]

Although we do not have a consistent naming convention for true/reconstructed energy, this is inheriting from previous experiments... So I guess what we did is enough.