Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Waveform noise tuner #1282

Merged
merged 21 commits into from
Aug 1, 2024
Merged

Waveform noise tuner #1282

merged 21 commits into from
Aug 1, 2024

Conversation

moralejo
Copy link
Collaborator

Code to compute the needed NSB tuning in the MC waveforms to match a certain data file.
It does not require to know the level of NSB simulated, or the level of electronic noise. It obtains the necessary noise addition in a purely empirical way.

lstchain/reco/r0_to_dl1.py Outdated Show resolved Hide resolved
@moralejo
Copy link
Collaborator Author

An example of tuning performed with this code, between
/fefs/aswg/data/real/DL1/20240627/v0.10/tailcut84/dl1_LST-1.Run17881.0030.h5
and
/fefs/aswg/data/mc/DL0/LSTProd2/TrainingDataset/GammaDiffuse/dec_2276/sim_telarray/node_corsika_theta_6.000_az_180.000_/output_v1.4/simtel_corsika_theta_6.000_az_180.000_run10.simtel.gz
image

@moralejo
Copy link
Collaborator Author

The code is not completely deterministic, since it applies the random noise (different levels) to compare it to the one in the data. I do not think this is a problem, but I guess @gabemery that one could fix all seeds to make it 100% reproducible

@moralejo moralejo requested a review from gabemery July 31, 2024 16:53
@moralejo
Copy link
Collaborator Author

moralejo commented Aug 1, 2024

And this is with the lstchain test file dl1_LST-1.Run02008.0100.h5 (only one ped event in the DL1 file, which is enough to roughly match the NSB noise):
image

So a meaningful test of the algo can be done with this new approach.
Unfortunately, the result of the tuning in the CI tests is completely different to what I get when I run it myself (hence the CI fails). Instead of (aprox) 3.2p.e. its gives 2.5 p.e. for the MC ped variance after tuning (this is well beyond the fluctuations from the randomness of the algorithm). I have no idea why, any suggestions?

I was just mixing up the two real data dl1 test files. The test works, it is just done with 2008.0000, not with 2008.00100.

@moralejo moralejo marked this pull request as ready for review August 1, 2024 09:53
Copy link

codecov bot commented Aug 1, 2024

Codecov Report

Attention: Patch coverage is 95.65217% with 5 lines in your changes missing coverage. Please review.

Project coverage is 73.49%. Comparing base (a3da2c0) to head (20d04bb).

Files Patch % Lines
lstchain/image/modifier.py 96.66% 3 Missing ⚠️
lstchain/reco/r0_to_dl1.py 93.75% 1 Missing ⚠️
lstchain/scripts/tests/test_lstchain_scripts.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1282      +/-   ##
==========================================
+ Coverage   73.44%   73.49%   +0.05%     
==========================================
  Files         134      134              
  Lines       14161    14207      +46     
==========================================
+ Hits        10400    10442      +42     
- Misses       3761     3765       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@morcuended morcuended left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No major comments, just some minor questions and suggestions

lstchain/reco/r0_to_dl1.py Outdated Show resolved Hide resolved
lstchain/image/modifier.py Show resolved Hide resolved
lstchain/image/modifier.py Outdated Show resolved Hide resolved
lstchain/image/modifier.py Outdated Show resolved Hide resolved
lstchain/image/modifier.py Outdated Show resolved Hide resolved
lstchain/image/modifier.py Outdated Show resolved Hide resolved
lstchain/image/modifier.py Outdated Show resolved Hide resolved
lstchain/image/modifier.py Show resolved Hide resolved
lstchain/image/modifier.py Show resolved Hide resolved
moralejo and others added 4 commits August 1, 2024 13:22
Co-authored-by: Daniel Morcuende <[email protected]>
Co-authored-by: Daniel Morcuende <[email protected]>
Co-authored-by: Daniel Morcuende <[email protected]>
Co-authored-by: Daniel Morcuende <[email protected]>
Copy link
Member

@morcuended morcuended left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no more comments from my side, but better let's @gabemery have a look

@moralejo moralejo merged commit 378a4c3 into main Aug 1, 2024
9 checks passed
@moralejo moralejo deleted the wf_noise_tuner branch August 1, 2024 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants