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

Coordinate System of photon_stream.point_cloud #44

Open
maxnoe opened this issue Oct 17, 2017 · 5 comments
Open

Coordinate System of photon_stream.point_cloud #44

maxnoe opened this issue Oct 17, 2017 · 5 comments

Comments

@maxnoe
Copy link
Member

maxnoe commented Oct 17, 2017

It seems the x and y coordinates are degrees and rotated 90 degrees to our reference coordinate system.

I would propose the following:
To be consistent with FACT-Tools, Mars and the original PixelMap file: keep the coordinates as is.
So x and y in mm and not rotated.

This is especially important for things like the reconstruction of origin, where the estimators need the cog in mm in this coordinate system and the coordinate trafo also expects this coordinate system.

I would like to reduce the number of coordinate systems to a bare minimum and introducing yet another (albeit probably more sane) creates a strong headache for me.

@relleums
Copy link
Member

relleums commented Nov 8, 2017

I agree that consistence is most important.
Especially the possible rotation by 90 degree is a problem.
The photon-stream uses this here:

https://github.com/fact-project/pyfact/blob/d38a708b5cea2faa7eeb6706f19ad34f73b5f6b4/fact/instrument/camera.py#L57

I do not understand yet where the 90 degrees rotation comes from, or where you have seen it.
I am also willing to drop the pincushion distortion if it makes thinking about the problem any more complicated.

@maxnoe
Copy link
Member Author

maxnoe commented Nov 8, 2017

The 90 degrees rotation comes from this function in respect to the pixel map file.

But I think it's better to use the rotated coordinate system (but in mm, not degrees).
See fact-project/fact-tools#249

@relleums
Copy link
Member

I thought about this again. I say that I will accept the wish of the majority, but personally I recommend to not use distances over direction angles. The photo-stream is meant to be a high level tool that is as decoupled from the telescope as possible (e.g. its focal length). Further using distances in units of mm instead of m leads only to surprises. All the photon-stream at the moment is base SI units. I just recently decided and made all angles to radians instead of degrees.
Another point is that the (to be honest small) imaging distortions of FACT only make any sense when the pixel directions are expressed using direction angles. When going to distances, we would have to drop this correction (which again to be honest is not a big correction 0.05deg for the outer most pixels).

@maxnoe
Copy link
Member Author

maxnoe commented Nov 27, 2017

All our other tools expect camera coordinates in mm. For example pyfact and the classifier tools.

@maxnoe
Copy link
Member Author

maxnoe commented Feb 28, 2018

As we are on the way of switching all our tools to the rotated coordinate system, this is not an issue anymore.

However, I would strongly urge to use mm for in camera positions.

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

2 participants