moralejo
commented
2 months ago Good point, although in practice if one gets values around 0 probably it is not worth to tune the NSB in the first place - I mean it will be a small tuning anyway, and the effect (compared to using dark MC) will be small. Rather than adding an option on what to return I think you can add an argument (with 0 as default) which is the minimum allowed value. You then set it to -100 or whatever if you want to get the negative values. The important thing is that the default behavior, without that extra argument, stays the same.
Elisa-Visentin
Hi, maybe this is more a discussion than an issue... In the calculate_noise_parameters function you default extra_noise_dim to 0 if you get a value < 0 https://github.com/cta-observatory/cta-lstchain/blob/378a4c3a8a1e72d8e3e1b6739edef9e591002261/lstchain/image/modifier.py#L406
In case you apply it only once to get the NSB level for a run, I agree that you cannot return a negative value (or you can return it but then you cannot remove noise from MC so you will have to put it to 0 later on). But in case you apply this function N times on the same run (i.e., using different subruns), if the average value is slightly above 0 you 'cut' one of the tails of the distribution, thus introducing a bias. What about adding an option (e.g., a boolean variable) to the function to allow both for the default behaviour (i.e., default to 0) and for a more 'custom' behaviour (i.e., return also negative values)?