Correct filling of photo_electron_image container in SimtelEventSource #943
Conversation
Codecov Report
@@ Coverage Diff @@
## master #943 +/- ##
=========================================
- Coverage 78.6% 78.6% -0.01%
=========================================
Files 191 191
Lines 10910 10909 -1
=========================================
- Hits 8576 8575 -1
Misses 2334 2334
Continue to review full report at Codecov.
|
|
thanks for catching that! It definitely would have broken some benchmarks. Yes, this points to a strong need to clean up the testing data. It will eventually be completely removed from ctapipe-extra, and be downloaded on-the-fly instead similar to gammapy. Right now I'm a bit afraid to remove gamma_test.simtel.gz, even though it's so outdated (prod2!), since a lot of the calibration tests seem to rely on specific values from it. Once those are make more general (e..g. with dummy inputs rather than a specific event), we can migrate. |
| data.mc.tel[tel_id].photo_electron_image = ( | ||
| array_event.get('photoelectrons', {}) | ||
| .get(tel_index, {}) | ||
| .get('photoelectrons', np.zeros(n_pixel, dtype='float32')) |
There was a problem hiding this comment.
is np.zeros correct here? or should it be np.nans since we have no idea what the values are ... instead of being sure they were zeros.
There was a problem hiding this comment.
I know it was also np.zeros before this PR .. so this PR is in principle not responsible for adressing this .. but still.
There was a problem hiding this comment.
np.zeros was what was returned in the case of HESSIOEventSource, but I don't have a strong opinion about what to return...
For normal simtel files (prod3+), the np.zeros will never be returned anyway. Only in the case of the prod2 test files
The data.mc.tel[tel_id].photo_electron_image container is not correctly filled. This is a difficult task to test, because for some reason the test files do not contain an array_event['photoelectrons'] table. This has been an issue with these test files for a long time.
This PR fixes the accessing of the photoelectron information from the simtel file. It is important to also note that the array_event['photoelectron'] keys correspond to telescope index, NOT telescope id.