search

cta-observatory / ctapipe

Low-level data processing pipeline software for CTAO or similar arrays of Imaging Atmospheric Cherenkov Telescopes
https://ctapipe.readthedocs.org
BSD 3-Clause "New" or "Revised" License
63 stars 266 forks source link

Calculation of Hillas Parameters in the Nominal System #674

Closed ParsonsRD closed 4 years ago

ParsonsRD commented 6 years ago

From looking through the code the default (only) way to calculate the Hillas parameters seems to be in the camera frame. Probably the default behaviour of this of this code should be to calculate the Hillas parameters in the Nominal shared angular system. Having these values in angular units makes a lot more sense I think as they are then inherently comparable between different telescope types, additionally any further reconstruction will take place in the Nominal system (or the Horizon system).

Although the Hillas parameters can always be converted between systems if a special routine it written (barring any complex non linear Camera to Nominal system conversion), I think having a Hillas with measured in metres is physically meaningless and we should never really want this.

My suggestion for a new behaviour to fix this and make coordinates easier to use in general would be to store both the pixel positions in the geometry object as a coordinate object and update the array and telescope pointing directions in this object for each event. That way the user can convert to any system they like with a single line without having to get values from the event.inst... object.

@kosack how does that sound to you?

kosack commented 6 years ago

Right now, the HillasReconstructor does not use the coordinate frames, so we have to think of the right way to do it without breaking things. I sort of want to wait until we have fixed the coordinates module, which doesn't quite do things correctly - I think somebody had mentioned they started migrating it?

However, in general, this is something that I've wanted to fix at some point - either adding a method to CameraGeometry to convert the pixel positions to other frames, or adding a function that allows us to transform the Hillas parameters themselves afterward (which may be more efficient).

ParsonsRD commented 6 years ago

OK that's fine. I agree we should come up with some code to convert Hillas parameters between systems, but I think probably the default behaviour should be to create them in some kind of angular system as the use cases of these values are many, whereas the use cases on Hillas parameters measured in metres are essentially none (drawing on top of the camera I guess).

I think it's good also to take a closer look at the coordinates code. I would be happy to contribute some time to this. Do we have a plan for how to change things? If not to we have a list of issues with the current implementation?

maxnoe commented 6 years ago

What is the Nominal System and is there any document that defines these frames?

ParsonsRD commented 6 years ago

This is a major part of the problem, no there isn't yet any documentation of what coordinate systems we should have/use. When I wrote this code I created the systems I though we would need with a heavy leaning on the HESS documentation, but my hope was that a coordinates guidelines document would be available soon after.

All the systems I created are currently only documented in their docstrings e.g. http://cta-observatory.github.io/ctapipe/api/ctapipe.coordinates.NominalFrame.html#ctapipe.coordinates.NominalFrame

maxnoe commented 4 years ago

Duplicate of #1090