ctapipe_io_magic issueshttps://gitlab.mpcdf.mpg.de/ievo/ctapipe_io_magic/-/issues2021-12-14T21:13:41Zhttps://gitlab.mpcdf.mpg.de/ievo/ctapipe_io_magic/-/issues/13Switching to ctapipe 0.122021-12-14T21:13:41ZFederico Di PierroSwitching to ctapipe 0.12Using the latest containers will allow to use all recent functions and scripts implemented in ctapipe.Using the latest containers will allow to use all recent functions and scripts implemented in ctapipe.ctapipe 0.12https://gitlab.mpcdf.mpg.de/ievo/ctapipe_io_magic/-/issues/14Missing CameraReadout in CameraDescription2021-12-06T07:55:07ZAlessio BertiMissing CameraReadout in CameraDescriptionIn the passage to ctapipe v0.12 (see #13), CameraDescription has an attribute which is a CameraReadout object. This class is filled from a file fits.gz, which is read whenever the CameraDescription is created with the method from_name (e...In the passage to ctapipe v0.12 (see #13), CameraDescription has an attribute which is a CameraReadout object. This class is filled from a file fits.gz, which is read whenever the CameraDescription is created with the method from_name (e.g. CameraDescription.from_name("MAGICCam") gives the MAGIC camera description). This file is not produced for MAGIC (the CameraReadout is a new class), so we should produce it and upload it to the CTA server where all this kind of data is stored.
An example for such fits.gz file is the one for LSTCam.
See also [https://github.com/cta-observatory/ctapipe/blob/v0.12.0/ctapipe/instrument/camera/readout.py](https://github.com/cta-observatory/ctapipe/blob/v0.12.0/ctapipe/instrument/camera/readout.py).
To create the needed file, in principle one could do also:
```python
from ctapipe.instrument import CameraReadout
import numpy as np
from astropy.table import Table
pulse_shape_lo_gain = np.array([...]) #fill with some pulse shape
pulse_shape_hi_gain = np.array([...]) #fill with some pulse shape
pulse_shape = np.vstack((pulse_shape_lo_gain, pulse_shape_lo_gain))
camera_readout = CameraReadout(
camera_name='MAGICCam',
sampling_rate=u.Quantity(1.64, u.GHz),
reference_pulse_shape=pulse_shape,
reference_pulse_sample_width=u.Quantity(0.5, u.ns)
)
camera_readout_table = camera_readout.to_table()
camera_readout_table.write("MAGICCam.camreadout.fits.gz")
```Alessio BertiAlessio Bertihttps://gitlab.mpcdf.mpg.de/ievo/ctapipe_io_magic/-/issues/10Switching from uproot3 to uproot42021-11-28T11:06:48ZFederico Di PierroSwitching from uproot3 to uproot4Some simple changes were necessary to use uproot4 (new default uproot version) instead of uproot3.
Basically some functions changed slightly their names (upper-lower cases: AsJagged and AsDtype) and it was necessary to explicitly set the...Some simple changes were necessary to use uproot4 (new default uproot version) instead of uproot3.
Basically some functions changed slightly their names (upper-lower cases: AsJagged and AsDtype) and it was necessary to explicitly set the library for filling the arrays (as: library="np", otherwise the default is "awkward" array).
Furthermore the array name string has to be a simple string instead of a byte string (e.g.: b'').
In the new branch [https://gitlab.mpcdf.mpg.de/ievo/ctapipe_io_magic/-/tree/dev-uproot4] these modifications are implemented.
Since the strength of awkward arrays could be a speed-up of the run time we could investigate how to fully exploit this new resource before merging to master.https://gitlab.mpcdf.mpg.de/ievo/ctapipe_io_magic/-/issues/11No interpolation of events az/zd/ra/dec gives error in astropy2021-01-15T10:04:19ZAlessio BertiNo interpolation of events az/zd/ra/dec gives error in astropyWhen running `hillas_preprocessing_MAGICCleaning_stereo.py` from https://gitlab.mpcdf.mpg.de/ievo/magic-cta-pipe (as of commit 8005e0914eb0910d2014cdc212ffe925b0e79861) repository on a run of OFF data (run 05085314 from 20190925, source ...When running `hillas_preprocessing_MAGICCleaning_stereo.py` from https://gitlab.mpcdf.mpg.de/ievo/magic-cta-pipe (as of commit 8005e0914eb0910d2014cdc212ffe925b0e79861) repository on a run of OFF data (run 05085314 from 20190925, source AT2019pqh), I get the error:
```
Traceback (most recent call last):
File "hillas_preprocessing_MAGICCleaning_stereo.py", line 493, in <module>
output_name=config['data_files'][data_type][sample][telescope_type]['hillas_output'])
File "hillas_preprocessing_MAGICCleaning_stereo.py", line 321, in process_dataset_data
frame=horizon_frame,
File "/storage/gpfs_data/ctalocal/aberti/miniconda3/envs/magic-lst/lib/python3.7/site-packages/astropy/coordinates/sky_coordinate.py", line 315, in __init__
frame_cls(**frame_kwargs), args, kwargs)
File "/storage/gpfs_data/ctalocal/aberti/miniconda3/envs/magic-lst/lib/python3.7/site-packages/astropy/coordinates/sky_coordinate_parsers.py", line 245, in _parse_coordinate_data
valid_components.update(_get_representation_attrs(frame, units, kwargs))
File "/storage/gpfs_data/ctalocal/aberti/miniconda3/envs/magic-lst/lib/python3.7/site-packages/astropy/coordinates/sky_coordinate_parsers.py", line 592, in _get_representation_attrs
valid_kwargs[frame_attr_name] = repr_attr_class(value, unit=unit)
File "/storage/gpfs_data/ctalocal/aberti/miniconda3/envs/magic-lst/lib/python3.7/site-packages/astropy/coordinates/angles.py", line 536, in __new__
self._validate_angles()
File "/storage/gpfs_data/ctalocal/aberti/miniconda3/envs/magic-lst/lib/python3.7/site-packages/astropy/coordinates/angles.py", line 558, in _validate_angles
'got {}'.format(angles.to(u.degree)))
ValueError: Latitude angle(s) must be within -90 deg <= angle <= 90 deg, got 91.0 deg
```
The error is thrown for an event for which ```event.pointing.array_altitude``` is set to 91 deg. Checking the zenith values of the events in that run, they are all around 40-45deg, so the value must come from somewhere else. Indeed, I could trace the strange value of altitude to ctapipe_io_magic, in particular to the portion of code where interpolation of zd/az/ra/dec is performed. Examining the code, the interpolation of the events zd/az/ra/dec is performed subrun-wise starting from the drive reports. In the case there are 2 or less drive reports in a subrun, zd/az/ra/dec are set to -1 for all events in that subrun (and there is no warning about that). The altitude is then calculated as ```90-zd```, which results in a value of 91deg for zd=-1, that is the value retrieved in the ```magic-cta-pipe``` script.
Indeed, in one of the subruns for M1, there is only 1 drive report, therefore interpolation is not performed.Alessio BertiAlessio Bertihttps://gitlab.mpcdf.mpg.de/ievo/ctapipe_io_magic/-/issues/12Finding mono and stereo events take too much time when loading events2021-01-15T10:04:18ZAlessio BertiFinding mono and stereo events take too much time when loading eventsWhen performing a profiling of ctapipe_io_magic (as of commit c676ef1cd40a5c7c8b41848b4d9fa90225b2f34c) taking in one run (composed of multiple subruns), I found out that >50% of the time is spent in finding the indices of mono and stere...When performing a profiling of ctapipe_io_magic (as of commit c676ef1cd40a5c7c8b41848b4d9fa90225b2f34c) taking in one run (composed of multiple subruns), I found out that >50% of the time is spent in finding the indices of mono and stereo events (```_find_mono_events``` and ```_find_stereo_events``` methods of the ```MarsRun``` class), see attached screenshot (produced with ```snakeviz``` and the profiling file attached).
From inspection of the two methods code, the bottleneck is due to the several ```for``` loops, which are looping over a consistent number of events. In this sense, there should be a huge boost if those loops are replaced by ```numpy``` functions, since the starting data structures are ```numpy``` arrays.
The script I tested (called ```test_ctapipe_io_magic.py```) is very simple:
```python
from ctapipe_io_magic import MarsRun, MAGICEventSource
onerun = MarsRun("/storage/gpfs_data/ctalocal/aberti/MAGIC_LST/Analysis/Crab_campaign/CrabNebula/2020-01-19/Calibrated_ON/20200119*05088541*.root")
```
and to create the profile file I run the script with:
```bash
python -m cProfile -o profile.prof test_ctapipe_io_magic.py
```
[test_ctapipe_io_magic.prof](/uploads/469dfbb300304d569cb89e6668ea1c97/test_ctapipe_io_magic.prof)
![ctapipe_io_magic_profiling_c676ef1cd40a5c7c8b41848b4d9fa90225b2f34c](/uploads/85b4fb12cec5af12d89c580236ae9c33/ctapipe_io_magic_profiling_c676ef1cd40a5c7c8b41848b4d9fa90225b2f34c.png)Alessio BertiAlessio Bertihttps://gitlab.mpcdf.mpg.de/ievo/ctapipe_io_magic/-/issues/7Switching to ctapipe 0.82020-12-22T15:54:20ZMoritz HuettenSwitching to ctapipe 0.8Update the reader to comply with ctapipe 0.8 container structure and astropy 4Update the reader to comply with ctapipe 0.8 container structure and astropy 4ctapipe 0.8Moritz HuettenMoritz Huettenhttps://gitlab.mpcdf.mpg.de/ievo/ctapipe_io_magic/-/issues/9Fill PointingContainer2020-12-18T17:37:37ZAlessio BertiFill PointingContainerIn ctapipe v0.8, the `TelescopePointingContainer` was replaced by `PointingContainer`. `PointingContainer` contains a `TelescopePointingContainer` instance, plus other members, see https://github.com/cta-observatory/ctapipe/blob/v0.8.0/c...In ctapipe v0.8, the `TelescopePointingContainer` was replaced by `PointingContainer`. `PointingContainer` contains a `TelescopePointingContainer` instance, plus other members, see https://github.com/cta-observatory/ctapipe/blob/v0.8.0/ctapipe/containers.py#L654 and beyond.
Currently, the MAGIC reader defines `pointing` as a `TelescopePointingContainer` (e.g. https://gitlab.mpcdf.mpg.de/ievo/ctapipe_io_magic/-/blob/master/ctapipe_io_magic/__init__.py#L381), so it fills only part of what should be the final container. Therefore, `pointing` should be defined as a `PointingContainer` and filled accordingly. Also, the `array_altitude` and `array_azimuth` members are needed later to be passed to the function calculating stereo parameters.ctapipe 0.8Alessio BertiAlessio Bertihttps://gitlab.mpcdf.mpg.de/ievo/ctapipe_io_magic/-/issues/8Timestamps calculated in MAGICEventSource/MarsRun2020-07-23T12:55:53ZYoshiki OhtaniTimestamps calculated in MAGICEventSource/MarsRunCurrently, the timestamps in MAGICEventSource are calculated as follows:
```python
event_mjd = event_times[b'MTime.fMjd']
event_millisec = event_times[b'MTime.fTime.fMilliSec']
event_nanosec = event_times[b'MTime.fNanoSec']
event_mjd ...Currently, the timestamps in MAGICEventSource are calculated as follows:
```python
event_mjd = event_times[b'MTime.fMjd']
event_millisec = event_times[b'MTime.fTime.fMilliSec']
event_nanosec = event_times[b'MTime.fNanoSec']
event_mjd = event_mjd + (event_millisec / 1e3 + event_nanosec / 1e9) / seconds_per_day
event_data['MJD'] = scipy.concatenate((event_data['MJD'], event_mjd))
```
So the timestamps are calculated in units of "day" (Modified Julian Day, MJD).
When I try to find the coincident events between MAGIC and LST1, I prefer to use the timestamps in unit of "sec" counted since the start of each day.
So in order to get such kind of format, I need to do the following calculation:
**timestamp_reco = (event_mjd - int(event_mjd)) * day2sec [sec] (day2sec = 60x60x24)**
Then, I compared the value with the one that I simply reconstructed as follows:
**timestamp_true = event_millisec * millisec2sec + event_nanosec * nanosec2sec
(millisec2sec = 1e-3, nanosec2sec = 1e-9)**
Then, sometimes I saw the following values, for example:
**case1:
timestamp_reco = 82314.38639627304 [sec]
timestamp_true = 82314.3863962 [sec]**
**case2:
timestamps_reco = 82314.39802993555 [sec]
timestamps_true = 82314.3980302 [sec]**
In case of 1, it is ok if I discard the order below 100 ns.
However, in case of 2, even the order of 100 ns is different, and it may affect the coincidence algorithm because the coincidence window is ~ 1 µs order.
Actually, this is my concern and why I would like to change the timestamp calculation in MAGICEventSource.
This problem seems to be related with the accuracy of the float..? I don't know.
So my proposal of changes is,
**1. simply store the mjd, millisec and nanosec parameters separately in MarsRun object**
**2. change the get_stereo_event_data function to extract these parameters**
Actually, what I would like to do is to avoid using the mjd unit as the timestamps.
I would like to change the above things in dev-yoshiki branch and will do the merge-request. It would be appreciated if you gave me some comments or suggestions on this.
Best regards,
Yoshiki
Moritz HuettenMoritz Huettenhttps://gitlab.mpcdf.mpg.de/ievo/ctapipe_io_magic/-/issues/6use event.dl0.tel[tel_id].trigger_type field2020-06-17T17:27:37ZMoritz Huettenuse event.dl0.tel[tel_id].trigger_type fieldMoritz HuettenMoritz Huettenhttps://gitlab.mpcdf.mpg.de/ievo/ctapipe_io_magic/-/issues/5To add the possibility to read the MAGIC Superstar files2020-06-17T13:31:57ZFederico Di PierroTo add the possibility to read the MAGIC Superstar filesTo add containers for higher level data and fill them with data from Superstar files.
This will allow to skip all steps from Calibrated to Superstar (relying on MARS for them).To add containers for higher level data and fill them with data from Superstar files.
This will allow to skip all steps from Calibrated to Superstar (relying on MARS for them).Alessio BertiAlessio Bertihttps://gitlab.mpcdf.mpg.de/ievo/ctapipe_io_magic/-/issues/3Code crashes when M2 file alone is read2020-01-29T08:44:31ZMoritz HuettenCode crashes when M2 file alone is read@yusuke discovered that ctapipe_io_magic crashes when M2 file alone is read - this bug applies for both data and MC files.@yusuke discovered that ctapipe_io_magic crashes when M2 file alone is read - this bug applies for both data and MC files.Moritz HuettenMoritz Huettenhttps://gitlab.mpcdf.mpg.de/ievo/ctapipe_io_magic/-/issues/4No stereo partner MC events read in single file2020-01-29T08:44:17ZMoritz HuettenNo stereo partner MC events read in single fileWhen a single (M1/M2) MC file is read, only "mono" events (with stereo id = 0) are passed by the reader, not all those events that do have a non-zero stereo id (but simply the other-telescope partner file is missing).When a single (M1/M2) MC file is read, only "mono" events (with stereo id = 0) are passed by the reader, not all those events that do have a non-zero stereo id (but simply the other-telescope partner file is missing).Moritz HuettenMoritz Huettenhttps://gitlab.mpcdf.mpg.de/ievo/ctapipe_io_magic/-/issues/2Filling non existing members of TelescopePointingContainer2020-01-21T08:24:30ZAlessio BertiFilling non existing members of TelescopePointingContainerIn different places of [\_\_init\_\_.py](ctapipe_io_magic/__init__.py) ([L263](ctapipe_io_magic/__init__.py#263), [L264](ctapipe_io_magic/__init__.py#264), [L378](ctapipe_io_magic/__init__.py#378), [L379](ctapipe_io_magic/__init__.py#379...In different places of [\_\_init\_\_.py](ctapipe_io_magic/__init__.py) ([L263](ctapipe_io_magic/__init__.py#263), [L264](ctapipe_io_magic/__init__.py#264), [L378](ctapipe_io_magic/__init__.py#378), [L379](ctapipe_io_magic/__init__.py#379), [L493](ctapipe_io_magic/__init__.py#493) and [L494](ctapipe_io_magic/__init__.py#494)), the variable `pointing` of the class `TelescopePointingContainer` tries to access the members `ra` and `dec`. But according to the definition of `TelescopePointingContainer` (see [definition](https://github.com/cta-observatory/ctapipe/blob/master/ctapipe/io/containers.py#L484)), the only defined members are `azimuth` and `altitude`.
Therefore, running `ctapipe_io_magic` on real MAGIC data will throw an error when trying to access those non existing members.https://gitlab.mpcdf.mpg.de/ievo/ctapipe_io_magic/-/issues/1MAGIC I/O incompatible with ctapipe 0.7.0.post98+git2c69cf12019-12-03T13:24:56ZMoritz HuettenMAGIC I/O incompatible with ctapipe 0.7.0.post98+git2c69cf1At some point in the last months, ctapipe has become incompatible with ctapipe_io_magic. At least as from 0.7.0.post98+git2c69cf1 on (or some commits earlier), the error occurs:
```
from ctapipe_io_magic import MAGICEventSource
event_...At some point in the last months, ctapipe has become incompatible with ctapipe_io_magic. At least as from 0.7.0.post98+git2c69cf1 on (or some commits earlier), the error occurs:
```
from ctapipe_io_magic import MAGICEventSource
event_factory = MAGICEventSource(input_url=file_mask)
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)
<ipython-input-5-dcf7476bb7e7> in <module>
----> 2 event_factory = MAGICEventSource(input_url=file_mask)
~/software/miniconda2/envs/cta-dev/lib/python3.6/site-packages/traitlets/traitlets.py in __new__(cls, *args, **kwargs)
953 new_meth = super(HasDescriptors, cls).__new__
954 if new_meth is object.__new__:
--> 955 inst = new_meth(cls)
956 else:
957 inst = new_meth(cls, *args, **kwargs)
TypeError: Can't instantiate abstract class MAGICEventSource with abstract methods subarray
```