Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IRF output files should have sufficient metadata #126

Open
kosack opened this issue Mar 1, 2021 · 4 comments
Open

IRF output files should have sufficient metadata #126

kosack opened this issue Mar 1, 2021 · 4 comments

Comments

@kosack
Copy link

kosack commented Mar 1, 2021

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

@maxnoe
Copy link
Member

maxnoe commented Mar 1, 2021

This is already possible, you can pass arbitrary header cards to the create HDU functions or edit the headers afterwards.

@maxnoe maxnoe closed this as completed Mar 1, 2021
@kosack
Copy link
Author

kosack commented Mar 1, 2021

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

@kosack kosack reopened this Mar 1, 2021
@maxnoe
Copy link
Member

maxnoe commented Mar 2, 2021

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.

@jsitarek
Copy link
Contributor

jsitarek commented Mar 2, 2021

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.

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

No branches or pull requests

3 participants