Fix original MC nsb reading #1272
Conversation
lstchain/io/io.py
Outdated
| tel_id = 1 | ||
| with SimTelFile(filename) as f: | ||
| try: | ||
| for _, line in f.history: |
There was a problem hiding this comment.
This seems very brittle. Don't we have a better way to understand which option is the correct one to use other than the one that follow STORE_PHOTOELECTRONS?
There was a problem hiding this comment.
I don't see one. But I am not an expert of simtel so maybe I am missing something. As far as I know the information is only located here and there is no clear indication of its usage.
What I checked is that, for whatever reason, the nsb entries corresponding to the final config of each telescope follow this pattern for the two latest MC productions (in the files I checked). But not other entries.
I agree it is brittle and the use of the new metadata available in Simtel will be better once we use it for MC productions. Until then, we have to hope that the config used are somewhat consistent...
There was a problem hiding this comment.
I see no better option as of now.
Perhaps you can add a clear warning log message stating what value will be used, so that the user can eventually check on the simtel config files that it is what it should be.
There was a problem hiding this comment.
Good idea, this info is currently given in a info level message without warning the user to check it. I will add a dedicated warning message.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1272 +/- ##
==========================================
- Coverage 73.28% 73.27% -0.01%
==========================================
Files 134 134
Lines 14084 14074 -10
==========================================
- Hits 10321 10313 -8
+ Misses 3763 3761 -2 ☔ View full report in Codecov by Sentry. |
|
Ok from my side! Unless @maxnoe has further comments, I am fine with this. |
|
Seems like this is the best we can do for now |
closes #1271
Change the behaviour of
extract_simulation_nsbto identify the nsb entries used with the telescopes and ignore other entries (for prod5 and LSTprod2). Return a dictionary to be accessed by tel_id.Replace the access to simtel object by hand in favour of
SimTelFile.history.