IRF output files should have sufficient metadata
Opened this issue · 4 comments
When writing IRF files in pyirf.io.gadf
format, it should be possible to add some extra metadata describing the context of the IRF and its observation parameters.
That will make it easier later to open and interpolate between IRFs. For that we should define an initial set of header keywords. Perhaps, reusing some standard FITS headers when possible, such as:
IRFTYPE = "fixed" / IRF type ("fixed" bin in phase space or "averaged" over obervation conditions)
ALTITUDE = 80.0 / altitude of simulation in degrees (zenith is then 90-ALTITUDE) [deg]
AZIMUTH = 180.0 / azimuth of simulation [deg]
NSB = 1 / NSB level multiplier (1=1x nominal)
Additionally, having the layout name, subarray name, prod name, etc would help avoid errors.
This will facilitate the bookkeeping that is necessary to do IRF interpolation, as in #124 #125
This is already possible, you can pass arbitrary header cards to the create HDU functions or edit the headers afterwards.
This should be demonstrated in the example notebooks then, as a best practice. Otherwise, it will be done in a non-standard way by people that use the library
I am very reluctant on adding any more stuff to the event display example. Even more, I'm inclined to completely remove it as an example and only have it in the tests.
It is not an example on how pyirf should be used, it was just a script to compare the results to Event Display.
People using pyirf should understand what the functions do and what the needed input / the output is. Not just follow this "example".
I also don't want to duplicate all the optional headers of the GADF specification or even add new conventions.
Or we need to rediscuss the scope of pyirf. In my understanding, pyirf should follow a specification (right now GADF) not create a new specification.
Hi @kosack the headers will be tricky, because it is difficult to know what we will want to have inside them in one year from now or so. Azimuth and Altitude sounds fine, but again it might happen that some simulations are done for points in the sky, while others for small ranges of Alt/Az. Also we should be very clear which azimuth we take, the Corsika one, or the geographical one, and if the range is -180 to 180 or 0-360
About NSB, I'm not sure if there is something like a "standard" NSB in CTA, there is some value assumed in the simulations, but it happens very often that such numbers are revised, and it might happen that in 1-2 years from now "standard" is not "standard" anymore. Also NSB changes somewhat with zenith, so if this change at some point get into "standard" it would again affect its definition.
Then there is a bunch of parameters describing the telescopes performance that would evolve in time, you want to store those somehow with the IRF to know what those files correspond to, but there are to many to put them one by one in headers. Nevertheless I agree with you that for the headers to have any practical usage at all, they have to be fixed to particular set of keywords and we cannot simple allow the user write there whatever they want with whatever tags
@maxnoe I think it would be a pity to remove this eventdisplay example. It is really useful to the users and it can be used as @kosack says to show best-practices in the headers, extension naming, etc.