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

Improve description of PSF axes #175

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

GernotMaier
Copy link

Proposal to clarify the meaning of the axes in the PSF histograms (see also issue #174)

Proposal to clarify the meaning of the axes in the PSF histograms (see also issue open-gamma-ray-astro#174)
@@ -18,9 +18,9 @@ Columns:
* ``ENERG_LO``, ``ENERG_HI`` -- ndim: 1, unit: TeV
* True energy axis
* ``THETA_LO``, ``THETA_HI`` -- ndim: 1, unit: deg
* Field of view offset axis (see :ref:`coords-fov`)
* Angular distance to centre of the array pointing direction (see :ref:`coords-fov`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AE vs BE? Do we have a guideline for that? I would say AE

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep "axis", as this refers to the axis definition of the matrix

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not say "Array", as this may be used by non-IACT instruments...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But then some one should change https://gamma-astro-data-formats.readthedocs.io/en/latest/general/coordinates.html#coords-fov , as here it is explicitly said that these types of coordinates are used for IACTs and that the centre of the FOV is the centre of the array pointing direction.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TarekHC I would also say that the issue of rewording everything so it also works e.g. for HAWC is a larger one and should probably be done by someone from the Water Cherenkov community.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are too many places where this is currently an issue.

* ``RAD_LO``, ``RAD_HI`` -- ndim: 1, unit: deg
* Offset angle from source position
* Angular distance between simulated and reconstructed direction
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add axis here to be consistent.

Change from BE to AE; added zxis
@maxnoe
Copy link
Member

maxnoe commented Apr 23, 2021

It's also not mentioned that these columns are lower and upper bin edges respectively. Maybe we should write it like this?


Columns:

* ``ENERG_LO`` -- ndim: 1, unit: TeV
    Lower bin edges of the true energy axis
* ``ENERG_HI`` -- ndim: 1, unit: TeV
    Upper bin edges of the true energy axis 
* ``THETA_LO`` -- ndim: 1, unit: deg
    Lower bin edges of the axis in the angular distance to center of the array pointing direction (see :ref:`coords-fov`)
*  ``THETA_HI`` -- ndim: 1, unit: deg
    Upper bin edges of the axis in the angular distance to center of the array pointing direction (see :ref:`coords-fov`)
* ``RAD_LO``  -- ndim: 1, unit: deg
     Lower bin edges of the axis in angular distance between simulated and reconstructed direction
* ``RAD_HI`` -- ndim: 1, unit: deg
    Upper bin edges of the axis in angular distance between simulated and reconstructed direction
* ``RPSF`` -- ndim: 3, unit: sr^-1
   Matrix of point spread function values in the binning defined in the above columns :math:`dP/d\Omega`, see :ref:`psf-pdf`.

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

Successfully merging this pull request may close these issues.

3 participants