ctapipe_io_magic issueshttps://gitlab.mpcdf.mpg.de/ievo/ctapipe_io_magic/-/issues2021-11-28T11:06:48Zhttps://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/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/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
```