Improvements to frames in CameraGeometry

[maxnoe]

With the introduction of the EngineeringCameraFrame, the CameraGeoemetry also gained a frame attribute describing the the frame in which the pixel coordinates are defined.

However, a few things could be improved:

• define the frame in the geometry fits files
• allow transformation into TelescopeFrame or even all other frames
• Make CameraDisplay work with CameraGeometry in TelescopeFrame, see Plotting events in TelescopeFrame #1186

This is also related probably to #1090, as it would be easy to do, by just transforming the CameraGeometry into TelescopeFrame and then passing to hillas_parameters.

[kosack]

Note that CameraDisplay and other displays could also have a frame attribute, and should convert the given geometry and parameters as needed

[dneise]

I have a conceptional problem here, or a problem of understanding ... or so.

TL;DR: I think (and might be wrong) our world would be better, if CameraGeometry and OpticsDescription were merged into TelescopeDescription and were hard to split apart.

My problem goes like this:

1.) CameraGeometry has a frame, by default it is None, but when needed it is actually CameraFrame().

2.) CameraFrame can get a bunch of parameters to initialize itself, and I do not really understand which of them are needed and which are optional. Apparently all are optional, but if one ever wants to transform into a TelescopeFrame then at least focal_length must not be zero (which happens to be its default, and it is the only sensible default, if you do not know anything better, I think).

3.) coming back to the default frame of CameraGeometry, which is CameraFrame(with focal_length=0). I think CameraFrame is a useful default, but more so, if one would know the focal_length at the time, when a default frame is needed. But the focal_length is stored in CameraGeometrys sister structure OpticsDescription.

In my understanding these structures are "sisters" since the live together in the same house, called TelescopeDescription. For all intends and purposes every user who needs to get access to a CameraGeometry, the user will get it from the enclosing TelescopeDescription. Some people have seen free living CameraGeometries in the wild, but only under very rare circumstances, like examples or so.

The reason for having only CameraGeometries in examples and not complete TelescopeDescriptions, in my understanding is convenience, it is easy to write:

geom = CameraGeometry.from_name("LSTCam")

and it is harder to write:

tel_des = TelescopeDescription.from_name("LST", "LSTCam")

since one needs to remember, two arbitrary strings (was it cam or Cam??) and the order of the strings.
But honestly, do we still need this? Is it still useful to be able to write?

tel_des = TelescopeDescription.from_name("SST-1M", "LSTCam")

I mean ... of course ... there are other ways to fix my issue, without merging these structures, which have been divided since the beginning, but to me it really feels strange, when I am inside a function and I have access to the instrument description ... but only part of it... the focal length is missing.

---

Anyway, I tried to think of a different way of fixing my problem, which is "how to transform a CameraGeometry (which is given in a CameraFrame) convenientry into a TeleskopeFrame?"
And I think Max gave the answer in his top post, since he wrote "define the frame in the geometry fits files". I might get him wrong here, but I assume, this means: do not set frame=None, but provide the correct CameraFrame during construction of the CameraGeometry. Now if we did this, then we needed to specify the focal_length (which is a member of OpticsDescription) during construction of any CameraGeometry and ultimately we would store the same information in two different places, but we cannot do that, it violates DRY.

[maxnoe]

do not set frame=None, but provide the correct CameraFrame during construction of the CameraGeometry.

Yes!

CameraFrame can get a bunch of parameters to initialize itself, and I do not really understand which of them are needed and which are optional. Apparently all are optional, but if one ever wants to transform into a TelescopeFrame then at least focal_length must not be zero (which happens to be its default, and it is the only sensible default, if you do not know anything better, I think).

This is a bit unfortunate, but I think necessary for two reasons.
First: CameraFrame needs all these attrinutes to really be a SkyCoord, meaning it identifies a fixed position in the sky. This is only possible when location, obstime, focal_length and pointing_direction are set.
Second: we have situations where we need to deal with conversion from CameraFrame to AltAz but don't have sensible obstimes (mcs) so always requiring them to be set is also not possible.

The semantics are: if something is not set, assume it is the same for both frames!

Astropy would allow us to transform a CamereCoordinate at obstime1, location1, ... to a CameraFrame
at obstime2, location2, .....
Basically answering the question: "Where would object X that was at obstime1 visible at camera coordinates x, y in telescope 1 be at obstime2 in the camera of telescope2."

That's very powerful, but seldomly needed here. We mostly need transformations from Camera to Telescope to AltAz for the same obstime, location, ... and such don't set them and jsut assume they are the same.

[dneise]

Right .. thanks!
sorry for the wall of text, let me try to rephrase:

• CameraGeometry knows nothing about a focal_length, but OpticsDescription does.
• for a useful CameraFrame construction, we need to store focal_length in CameraGeometry.

--> I think this "forces" us to merge CameraGeometry & OpticsDescription due to DRY principles.

Do you agree Max?

[kosack]

For me, it's just that we pass info to the coordinate transforms in the wrong order. CameraFrame should not require any attributes (focal len, etc). It's purely meters on the camera, and should work fine. When you want to go to the TelescopeFrame, the telescope has a focal length, and should be an attribute of that frame (not something you add to CameraFrame, since the camera does not have a focal length!). You can put the same camera on 2 telescopes with different focal lengths.

see this comment on #416: #416 (comment)

[kosack]

To be more clear: CameraGeometry, hillas parameterization, etc, should all work fine without knowing anything about the telescope, even if you start to analyze data that is taken in the lab (which we will do ), where there is no telescope.

[kosack]

If an algorithm needs the focal length, it should receive a TelescopeDescription, not just a CameraGeometry.

Which means that perhaps we just need to provide a TelescopeDisplay that can do things in the TelescopeFrame or NominalFrame, similar to the CameraDisplay that does things in the CameraFrame)

[maxnoe]

For me, it's just that we pass info to the coordinate transforms in the wrong order. CameraFrame should not require any attributes (focal len, etc). It's purely meters on the camera, and should work fine. When you want to go to the TelescopeFrame, the telescope has a focal length

That's not how this works!

To be a valid coordinate, a frame needs all the information so it really represents a fixed coordinate in the sky.

So for a coordinate in x,y in the camera, you need the telescopes focal length, it's position and pointing position and the time.

So a CameraFrame needs to be able to store all those attributes.

[kosack]

Is that a requirement of SkyCoord? Or just the way we are using it?

[maxnoe]

A coordinate in CameraFrame is not a SkyCoord in any semantical way if this does not hold true and you will not be able to do direct transformations from camera_frame or telescope_frame to ICRS if this is not the case.

Right now, we can go from CameraFrame to ICRS directly. If we remove focal_length and pointing_position attribute from the CameraFrame, each intermediate step would have to be done explicitly and each intermediate frame would have to be instantiated with its respective attributes.

I agree that having a focal length at the CameraFrame is a bit unintuitive at first.
But how I understood it and how it perfectly works at the moment is:
A frame must store all attributes that you need to perform transformation to ICRS.

[kosack]

Ok, I see the problem then - it is a but unintuitive.

Still I'd prefer not merging Camera and Telescope objects. We could put the frame in the TelescopeDescription, perhaps?

[kosack]

I guess that means we also have a temperature and pressure attribute of CameraFrame? (since those can be used by the transform from Alt/Az to Ra/Dec for refraction?). I.e. all attributes of frames between it and ICRS are inherited?

[maxnoe]

No at the moment we don't have that.
To do that, you would need to transform to an intermediate AltAz having those attributes set and then to ICRS.

But refraction corrections are on the order of mas, are they not?

[dneise]

We could put the frame in the TelescopeDescription, perhaps?

This is functionally what I was asking for, no? If we merge the two constituents of TD into one another, or attach the frame to the TelDes ... results in the same, no? Every decent function, will from then moment on ask for a TelDes instead of a CamGeom, if it ever wants to be able to be used within a different frame, no?

the next thing, which will happen ... people will ask for convenience properties, since teldes.camera.pix_x is longer than teldes.pix_x. The next might be .. people realize that in virtually all cases where a TelDes is being constructed ... first both its constituents need to be made, and then combined into a TelDes .. so they might ask for a convenient init to do all in one.... but maybe I am wrong here.

[dneise]

You can put the same camera on 2 telescopes with different focal lengths.

Yes, that is true. I think we are all aware of the reasoning behind the current implementation. I did not mean to invalidate that reasoning. I was merely trying to ask, if there are other arguments, which might weigh heavier, which might point towards a functional merging of ImageSensorGeometry and OpticsDescription into a true CameraDescription.

It is true, that people in the lab, will want to try their image sensors without a lens ... or mirror, but does it really hurt them if the description object contains additional information, which allowed them to transform their image from the EngineeringFrame into a TelescopeFrame and see how the image would look, had they mounted their sensor on a telescope, and had pointed it somewhere? I think it hardly hurts, but it sounds very useful to me...

[kosack]

If we merge the two constituents of TD into one another

Why merge them? TelescopeDescription is exactly that! It's a merge of the optics and camera. There may be other things added later - if we put them all in one class, it will grow to something unusable.

[kosack]

teldes.camera.pix_x I think is pretty clear, as is teldes.optics.mirror_facet_x (or similar). Rememebr that eventually this will be used to define MC simulations, so it will grow into a fairly complex class. It may also have teldes.drive.x. We could shorten some things, like go for teldes.camera.x rather than pix_x, but it's not super clear that that is not the x-position of the camera, rather than each pixel.

[kosack]

One more thing: we are also forgetting astrometric corrections, which are an input to the coordinate transformations as well. Those are clearly a function of the telescope optics (not the camera), drives, etc. So the question is, is it ok to have some attributes of a Frame not filled in until they are known? I think yes, since there are many cases where you want to only work at a low level (like parameterization), or where you don't need light-bending corrections, or astrometric corrections.

Of course, one could say those are optional attributes, whereas focal length is require. Anyhow, I think the semantic problem could just be solved by giving the Display a TelescopeDescription.

[dneise]

I think the semantic problem could just be solved by giving the Display a TelescopeDescription.

I do agree ... if we also move the frame from the the ImageSensorDescription to the TelescopeDescription, right?

[maxnoe]

Now that I think about it, we need to have a way to nicely define the CameraFrame and later set the focal_length and poiting_position attributes.

[kosack]

see: https://docs.astropy.org/en/stable/coordinates/inplace.html

[kosack]

I do agree ... if we also move the frame from the the ImageSensorDescription to the TelescopeDescription, right?

Yes. Of course, if instead we keep the frame in CameraGeometry, we should just go all in and store the pix_x/y coordinates as SkyCoords.

[maxnoe]

That concerns mutating the data, not the attached frame. For that:

c = c.replicate(obstime=t, location=lp)

Does not copy the data but updates the properties of the frame

[dneise]

I am lost.

[maxnoe]

I think this was finally implemented by a series of PRs,

• Add correct frame to CameraGeometry in SubArrayDescription.from_hdf, fixes #1469 #1476
• Fix CameraGeometry coordinate conversion for spherical frames #1407