search

cta-observatory / ctapipe

Low-level data processing pipeline software for CTAO or similar arrays of Imaging Atmospheric Cherenkov Telescopes
https://ctapipe.readthedocs.org
BSD 3-Clause "New" or "Revised" License
64 stars 268 forks source link

Inconsistent LSTCam pixels ordering when importing using CameraGeometry.from_name #2484

Closed abm017 closed 6 months ago

abm017 commented 9 months ago

The LSTCam pixel positions ordering obtained using CameraGeometry.from_name('LSTCam') differs from the ordering obtained using subarray.tel[1].camera.geometry.pix_x

Consequently, using CameraGeometry.from_name('LSTCam') leads to incorrect LST images as the pixels are arranged differently in the camera plane.

To Reproduce Steps to reproduce the behavior:

  1. ctapipe.__version__ : 0.19.3
  2. Compare CameraGeometry.from_name('LSTCam').pix_x and tel.camera.geometry.pix_x (output of latter can also be seen in this tutorial -https://ctapipe.readthedocs.io/en/latest/auto_examples/core/InstrumentDescription.html#explore-some-of-the-details-of-the-telescopes)

Expected behavior The ordering should be same in both cases (it is same for other telescopes (FlashCam, NectarCam, CHEC)).

Supporting information Simple plots of pix_x and pix_y positions from both methods.

image image

Additional context

kosack commented 9 months ago

This is not a bug, but expected behavior. Constructing a CameraGeometry using CameraGeometry.from_name() loads a geometry that was uploaded to the test server for unit tests, and shouldn't be relied upon for general analyses, as we cannot know if it corresponds to the data you are working with. The testing geometries are currently generated from Prod3b simulations, so may not be the same pixel ordering for other simulation productions or for real data. In fact, it's almost certain that the simulated pixel numbering is not the same as the physical cameras.

There are two solutions:

1) do not use from_name() for data processing, but instead rely on the geometry obtained from the EventSource (for any supported data file) or TableLoader (for ctapipe HDF5 files only), which if implemented correctly for the given input file format, should provide the correct CameraGeometry that corresponds to the data that are loaded.

2) set the environment variable CTAPIPE_SVC_PATH to include a directory containing the correct geometry FITS files (*.camgeom.fits.gz as generated by e.g. ctapipe-dump-instrument), which will then be the ones loaded by from_name() instead of the defaults from the test server.

Solution 1 is preferred, as it should work in all situations and doesn't rely on the user knowing which geometry corresponds to the data being processed.

abm017 commented 9 months ago

Ah ok, I did not know of the origin of the geometry that comes from using from_name(). It was just surprising that this only affected LSTCam and none of the other cameras. No bug/issue in that case. Maybe Solution 1 can be included in the documentation so that this pitfall can be avoided in general? (https://ctapipe.readthedocs.io/en/latest/api/ctapipe.instrument.CameraGeometry.html#ctapipe.instrument.CameraGeometry.from_name) :)

kosack commented 9 months ago

I added some info to the docstring in #2485 to be more clear, since this is not obvious, I agree!

kosack commented 9 months ago

I guess once we have stable hardware and simulations, we could ensure that the loaded instrument info is the same, but for now, we keep changing things so that won't work! Originally I had envisioned the ability to switch between different sets of test data as in #738, but so far that's not implemented. In any case, it's always best to rely on reading the data from the exact file you are using. For observed, DL0 data products which currently do not contain this info, we will have to develop a solution.

kosack commented 9 months ago

We may also just remove these methods now that we have real and simulated data to work with (#1978)