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

Calculation of Hillas Parameters in the Nominal System #674

Closed
ParsonsRD opened this issue Mar 12, 2018 · 5 comments
Closed

Calculation of Hillas Parameters in the Nominal System #674

ParsonsRD opened this issue Mar 12, 2018 · 5 comments

Comments

@ParsonsRD
Copy link
Member

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
Copy link
Contributor

kosack commented Mar 12, 2018

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
Copy link
Member Author

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
Copy link
Member

maxnoe commented Mar 13, 2018

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

@ParsonsRD
Copy link
Member Author

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
Copy link
Member

maxnoe commented Nov 25, 2019

Duplicate of #1090

@maxnoe maxnoe marked this as a duplicate of #1090 Nov 25, 2019
@maxnoe maxnoe closed this as completed Nov 25, 2019
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