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

Fix transform of CameraGeometry #2279

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Fix transform of CameraGeometry #2279

wants to merge 5 commits into from

Conversation

kosack
Copy link
Contributor

@kosack kosack commented Mar 2, 2023

Allow a Frame class to be passed so attributes are retained

Fixes #2277

The following now works:

from ctapipe.instrument import SubarrayDescription
from ctapipe.coordinates import CameraFrame, EngineeringCameraFrame
import astropy.coordinates as c

sub = SubarrayDescription.read("dataset://gamma_prod5.simtel.zst")
geom= sub.tel[100].camera.geometry
newgeom = geom.transform_to(EngineeringCameraFrame)
print(geom.frame)
print(newgeom.frame)
<CameraFrame Frame (focal_length=16.445049285888672 m, rotation=0.0 rad, telescope_pointing=None, obstime=None, location=None)>
<EngineeringCameraFrame Coordinate (focal_length=16.445049285888672 m, rotation=0.0 rad, telescope_pointing=None, obstime=None, location=None, n_mirrors=1): (x, y) in m
    [( 0.   ,  0.   ), ( 0.   , -0.049), (-0.049,  0.   )]>

Note that the focal length and other parameters are retained in the transform

- Allow a Frame class to be passed so attributes are retained
@kosack
Copy link
Contributor Author

kosack commented Mar 2, 2023

TelescopeFrame will still lose the info, but I guess that is fundamentally because it doesn't contain that attribute.

sub = SubarrayDescription.read("dataset://gamma_prod5.simtel.zst")
geom = sub.tel[100].camera.geometry
print(geom.frame)
print(geom.transform_to(EngineeringCameraFrame).frame)
print(geom.transform_to(TelescopeFrame).frame)
<CameraFrame Frame (focal_length=16.445049285888672 m, rotation=0.0 rad, telescope_pointing=None, obstime=None, location=None)>
<EngineeringCameraFrame Coordinate (focal_length=16.445049285888672 m, rotation=0.0 rad, telescope_pointing=None, obstime=None, location=None, n_mirrors=1): (x, y) in m
    [( 0.   ,  0.   ), ( 0.   , -0.049), (-0.049,  0.   )]>
<TelescopeFrame Coordinate (telescope_pointing=None, obstime=None, location=None): (fov_lon, fov_lat) in deg
    [(0.        , 0.        ), (0.        , 0.17071965),
     (0.17071965, 0.        )]>

However, if the original CameraFrame has an obstime or telescope_pointing, they is retained, as they are attributes of TelescopeFrame as well. I wonder if we should add focal_length to TelecopeFrame, so full round-trips are possible?

@kosack
Copy link
Contributor Author

kosack commented Mar 3, 2023

from the Astropy docs:

The same lower-level frame classes also have a transform_to() method that works the same as above, but they do not support attribute-style access. They are also subtly different in that they only use frame attributes present in the initial or final frame, while SkyCoord objects use any frame attributes they have for all transformation steps. So SkyCoord can always transform from one frame to another and back again without change, while low-level classes may lose information and hence often do not round-trip.

@kosack
Copy link
Contributor Author

kosack commented Mar 10, 2023

as mentioned in the issue, I think I will modify this to store the pix_x, pix_y coordines internally in a SkyCoord, rather than constructing a SkyCoord during transform. That will allow all frame attributes to be propegated, even ones not explicitly in the target frame. That means one will be able to transform from CameraFrame to TelescopeFrame and back again without re-defining the parameters.

@kosack kosack marked this pull request as draft March 10, 2023 13:46
@kosack kosack mentioned this pull request Sep 18, 2023
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.

transforming geometry loses frame attributes
1 participant