WIP: put TelescopeDescription input inst.subarray

[dneise]

@watsonjj spotted a bug in the SST1MEventSource.

The problem came up in connection with the new bokeh plotter in #714.

As far as I understood, EventSources are supposed to fill also the inst Field, which was not done by the SST1MEventSource. This PR tries to solve the problem.

Unit test still missing

• check that pixel ordering is indeed identical to the ordering we expect. (Not yet clear how to do that)

In digicampipe the pixel positions are read from a file named camera_config

In ctapipe the camera definition seems to be created from the file DigiCam.camgeom.fits.gz

[codecov]

Codecov Report

Merging #720 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #720      +/-   ##
==========================================
+ Coverage   68.14%   68.15%   +<.01%     
==========================================
  Files         196      196              
  Lines       10546    10548       +2     
==========================================
+ Hits         7187     7189       +2     
  Misses       3359     3359

Impacted Files	Coverage Δ
ctapipe/io/sst1meventsource.py	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c7d73cd...23635a9. Read the comment docs.

[watsonjj]

It should be noted that ctapipe contains no information (yet) about the pixel positions, and relies on them being supplied to the TelescopeDescription:

        teldesc = TelescopeDescription.guess(*pix_pos, foclen)
        data.inst.subarray.tels[chec_tel] = teldesc

For HESSIOEventSource the pixel positions are read from the hessio file.

For the TargetIOEventSource I utilise TargetCalib which contains the pixel positions for each pixel.

I'm not sure what options you have available to you, but perhaps one option is a HDF5 with the pixel positions stored inside?

I believe in the future the pixel positions will be stored in a central CTA instruments database, and we will not need all these unique solutions.

[dneise]

Thanks for your comment.

I am however a bit puzzled by: "ctapipe contains no information (yet) about the pixel positions".
If I try this:

In [1]: import matplotlib.pyplot as plt
In [2]: from ctapipe.instrument import TelescopeDescription
In [3]: tel_desc = TelescopeDescription.from_name('SST-1M', 'DigiCam')
In [4]: plt.plot(-tel_desc.camera.pix_y, -tel_desc.camera.pix_x, 'd')
In [5]: plt.savefig('digicam_x_is_minus_y_and_y_is_minus_x.png')

I get this result ...

....which looks very much like the camera view, which is created by the digicam viewer which currently exists in digicampipe: https://github.com/cta-sst-1m/digicampipe#example-viewer.

However I had to do play around with the axes like this x=-pix_y, y=-pix_x to get exactly the same orientation.

So ... may it be your information about missing pixel positions is outdated?

[watsonjj]

Ah! Very true, my knowledge is outdated.

I assume these are the pixel positions extracted from MC. The only caution I have then is that the pixel ordering of MC might not be the same as the pixel ordering inside SST1M (it certainly isn't for CHEC), so you should check that.

[dneise]

Yes I have to check that

[dneise]

@watsonjj you are right, just looking at the first 3 pixels shows clearly, that in digicampipe and in ctapipe the pix_id does not mean the same, but it seems also the area is a bit different. So maybe the positions are not only in a different order, but also shifted a bit ... I have to investigate that.

I am not sure how to proceed here. In CameraGeometry.from_name it is possible to specify a version. So I could simply create a new version of the Digicam.geomcam.fits file and read that one instead of the old one...

ctapipe/ctapipe/instrument/camera.py

Line 209 in c7d73cd

def from_name(cls, camera_id='NectarCam', version=None):

Alternatively I could just try to find the a mapping between the pixel ids, so that the result is closest to the result I would get in digicampipe...

I am confused.

---

from digicampipe.utils import DigiCam
digicampipe_geometry = DigiCam.geometry.to_table()
from astropy.table import Table
ctapipe_geometry = Table.read('DigiCam.camgeom.fits')
digicampipe_geometry[:3]

<Table length=3>

pix_id	pix_x	pix_y	pix_area
	mm	mm
int64	float64	float64	float64
0	12.15	-498.05	400.0
1	-24.3	-477.01	400.0
2	0.0	-477.01	400.0

ctapipe_geometry[:3]

<Table length=3>

pix_id	pix_x	pix_y	pix_area
	m	m	m2
int64	float64	float64	float64
0	0.4837000072	-0.0118000004441	0.000482051264597
1	0.463270008564	-0.0472000017762	0.000482051264597
2	0.463270008564	-0.0236000008881	0.000482051264597

[dneise]

I assume there is some historical evolution involved here which predates my time in SST1M, I will have to check this.

[dneise]

I think I made a mistake regarding the ids ... it seems the ids are in fact pretty much okay (after some shuffling around with the axes) ... however the positions are not the same.
I've opened an issue in the SST1M repo, I believe is followed by people who might know more about it.
cta-sst-1m/CTS#8

[kosack]

Yes, the EventSource should for now fill the SubarrayDescription

[watsonjj]

Is zero the correct index? Correct me if I'm wrong, but isn't event.telescopeID in fill_R0Container_from_zfile_event 1?

[watsonjj]

Do you think you could also make the same updates to NectarcamEventSource in this PR?

[kosack]

it might be better to pass this to the SubarrayDescription constructor instead, but this should work for existing code. If in the future we pre-compute some things in the constructor, however, it will fail.

e.g. this may be safer:

event.inst.subarray = SubarrayDescription(
    name="SST1MTestBenchArray", 
    tel_positions={0: [0,0]*u.m}, 
    tel_descriptions={0:TelescopeDescription.from_name("SST-1M","DigiCam")}
)

or similar

[kosack]

Also, using TelescopeDescription.from_name() of course assumes the DigiCam.camgeom.fits.gz and optics.ecsv.txt files in your CTAPIPE_SVC_PATH (or ctapipe-extra package) correspond to the input file your give (i.e. the pixel numbers are correct, etc). Since currently that was generated from Monte-Carlo, it may not be exactly right, so just keep that in mind. You might add a check in the code to make sure at least the number of pixels, etc, makes sense in case you load a file with a slightly modified camera geom.

[kosack]

So ... may it be your information about missing pixel positions is outdated?

Not outdated, but there is no guarantee the MC pixel positions correspond to your testbench camera pixels. The MCs, as you found out use a very strange coordinate system where x and y are swapped for some reason, with North along the y-axis, and we don't yet support conversion between this "MCCameraFrame" and the "EngineeringCameraFrame", which is probably more like what you get if you look at the front of the camera, rather than through it from the back as in the MCs.

So the point is if you use SST1m data and the default camera loaded up, which was generated from MCs, you may have a mismatch in pixel ids and camera rotation.

[kosack]

I merged this for now, I think you can add tests afterward - the basic functionality is there

[maxnoe]

I assume these are the pixel positions extracted from MC. The only caution I have then is that the pixel ordering of MC might not be the same as the pixel ordering inside SST1M (it certainly isn't for CHEC), so you should check that.

Good to hear that we (FACT) are not the only Collaboration with this :D