open-gamma-ray-astro / gamma-astro-data-formats

Data formats for gamma-ray astronomy
https://gamma-astro-data-formats.readthedocs.io
Creative Commons Attribution 4.0 International
30 stars 27 forks source link

Triple-Gauss PSF format #21

Open cdeil opened 8 years ago

cdeil commented 8 years ago

I've added a formula for the triple-Gauss PSF: http://gamma-astro-data-formats.readthedocs.org/en/latest/irfs/psf/psf_3gauss/index.html as well as some background info on radial PSFs: http://gamma-astro-data-formats.readthedocs.org/en/latest/irfs/psf/index.html#point-spread-function

I think some discussion / checks are needed:

cdeil commented 8 years ago

@JouvinLea commented via email:

why do you have S and not a normalization factor 1/(1pi_(s1_2 + A2 * S2_2 + A3_S3*2))? I mean the PSF has to be normalize no?

I agree, having a normalised PSF is nicer. The only reason to maybe not do it is backwards-compatibility with existing exporters / data / tools.

Thoughts?

cdeil commented 8 years ago

@jknodlseder - Could you please comment on the following questions concerning the triple-Gauss PSF format?

I think for the coming months / years we need some scheme to do backwards-incompatible changes to DL3 formats anyways to avoid being stuck with early mistakes, and we could use this as a test case on how to do this? But of course this only makes sense if it's agreed that the change is good, i.e. the new PSF parameterisation is an improvement.

jknodlseder commented 8 years ago

The SCALE parameter is in fact not used by GammaLib as the GCTAPsf2D class assures internally the normalisation. So in principle the SCALE parameter can be dropped. For the moment, the order of the parameter blocks in the FITS file are hardwired in GCTAPsf2D which is anyways a bad idea. Enhancing GCTAPsf2D so that the required parameter blocks are dynamically extracted from the column names would handle the two formats transparently. So we would not need to use the version header keyword for that (and to increment the number).

You may create a change request asking for an automatic detection of the parameter blocks based on column names. Note that this implies that the column names will then be fixed.

We also need to change GCTAResponseIrf::load_psf which for the moment checks the presence of columns and infers from that whether to use a Gaussian or a King PSF. We just need to remove the table.contains("SCALE") condition.

Le 2 févr. 2016 à 12:37, Christoph Deil a écrit :

@jknodlseder - Could you please comment on the following questions concerning the triple-Gauss PSF format?

Do you think changing to a parametrisation that is automatically normalised to integrate to 1 (like the King PSF) is a good idea? This would mean dropping SCALE as a parameter / column and putting the right normalisation factor in front of the format. Would this change still be acceptable in Gammalib? Maybe via a "version=2" header keyword? I think for the coming months / years we need some scheme to do backwards-incompatible changes to DL3 formats anyways to avoid being stuck with early mistakes, and we could use this as a test case on how to do this? But of course this only makes sense if it's agreed that the change is good, i.e. the new PSF parameterisation is an improvement.

— Reply to this email directly or view it on GitHub.

cdeil commented 8 years ago

@jknodlseder - Thank you for the quick reply!

Change request for Gammalib is here: https://cta-redmine.irap.omp.eu/issues/1656

We also need to change GCTAResponseIrf::load_psf which for the moment checks the presence of columns and infers from that whether to use a Gaussian or a King PSF. We just need to remove the table.contains("SCALE") condition.

I would much prefer to have declarative scheme. E.g. HDUCLASS and maybe a version as e.g. here: http://gamma-astro-data-formats.readthedocs.org/en/latest/irfs/background/index.html#bkg-3d-format

Having to maintain code that pokes and guesses formats in Gammalib / Gammapy for the coming years is painful and not really needed if we tell data producers via these specs what to put, right? (of course, short-term we have to keep doing this to keep the tools working with existing files...)

jknodlseder commented 8 years ago

I fully agree that a declarative scheme should be implemented.

As we had so far no conventions, we simply put the emphasize on a system that "works", but now that the conventions start to emerge, we should of course fully support them.

Le 3 févr. 2016 à 11:02, Christoph Deil a écrit :

@jknodlseder - Thank you for the quick reply!

Change request for Gammalib is here: https://cta-redmine.irap.omp.eu/issues/1656

We also need to change GCTAResponseIrf::load_psf which for the moment checks the presence of columns and infers from that whether to use a Gaussian or a King PSF. We just need to remove the table.contains("SCALE") condition.

I would much prefer to have declarative scheme. E.g. HDUCLASS and maybe a version as e.g. here: http://gamma-astro-data-formats.readthedocs.org/en/latest/irfs/background/index.html#bkg-3d-format

Having to maintain code that pokes and guesses formats in Gammalib / Gammapy for the coming years is painful and not really needed if we tell data producers via these specs what to put, right? (of course, short-term we have to keep doing this to keep the tools working with existing files...)

— Reply to this email directly or view it on GitHub.