cta-observatory / cta-lstchain

LST prototype testbench chain
https://cta-observatory.github.io/cta-lstchain/
BSD 3-Clause "New" or "Revised" License
22 stars 77 forks source link

monitoring tables in merged DL1 files are just those of the first merged subrun #1218

Closed moralejo closed 2 weeks ago

moralejo commented 5 months ago

The tables are not the result of merging the subrun-wise ones, but seem to be just a copy of the first subrun's.

The first row of the monitoring tables is the same for all subruns (corresponds to the results from the pedcal file), the rest are from the analysis of interleaveds. So a simple stacking of the rows would not make sense either.

This is probably a known issue. It has no consequences for the correctness of the post-DL1 analysis. And very likely the developments in the cat-B calibration makes this obsolete - to be confirmed by @FrancaCassol (close issue if that is the case)

morcuended commented 4 months ago

related to https://github.com/cta-observatory/cta-lstchain/issues/1193, which is giving problems for the DL1 runwise merging

morcuended commented 1 month ago

This long-standing issue keeps causing problems with the merging of onsite analysis products.

What do you suggest then @moralejo @FrancaCassol? Do not copy but merge Cat-A monitoring tables? Skip the checks that all should be the same when we have seeing that it shouldn't be like that #1193?

Remove completely this check of having the same length? https://github.com/cta-observatory/cta-lstchain/blob/9e2b2b04ca9c93492925d98e427681625e245a88/lstchain/io/io.py#L357-L362

Remove the Cat-A monitoring tables from the keys to directly copy from the first subrun?

moralejo commented 1 month ago

The check makes no sense, you can remove it. Also, this info is not used/needed in merged files, so I think we should not have even have the monitoring table in the merged files. Or have one per subrun (or one big table with an additional column subrun), if we want to keep it "just in case", but I think that is all obsolete with Cat-B

morcuended commented 1 month ago

check removed in #1256

morcuended commented 2 weeks ago

After #1256 these tables are no longer merged nor copied from the first subrun