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

Correct LSTR0Corrections cards for calibration script #432

Merged

Conversation

FrancaCassol
Copy link
Collaborator

Keep default trailets values for the selection of the R0 waveform samples to be used for the R1 waveform. Before the firmware upgrade of November, values were
"r1_sample_start": 2,
"r1_sample_end": 38

After, they should be:
"r1_sample_start": 3,
"r1_sample_end": 39

A test did not show major changes in the calibration results

@codecov
Copy link

codecov bot commented Jun 15, 2020

Codecov Report

Merging #432 into master will increase coverage by 0.17%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #432      +/-   ##
==========================================
+ Coverage   41.56%   41.74%   +0.17%     
==========================================
  Files          76       77       +1     
  Lines        6192     6211      +19     
==========================================
+ Hits         2574     2593      +19     
  Misses       3618     3618              
Impacted Files Coverage Δ
lstchain/visualization/plot_drs4.py 0.00% <ø> (ø)
lstchain/reco/tests/test_r0_to_dl1.py 100.00% <0.00%> (ø)
lstchain/reco/r0_to_dl1.py 64.48% <0.00%> (+0.27%) ⬆️
lstchain/calib/camera/calibration_calculator.py 27.39% <0.00%> (+0.37%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 82c294c...eb173e6. Read the comment docs.

@moralejo moralejo mentioned this pull request Jun 24, 2020
rlopezcoto
rlopezcoto previously approved these changes Jun 25, 2020
@morcuended
Copy link
Member

I realized that the old values are still in some notebooks and in this visualization script, cta-lstchain/lstchain/visualization/plot_drs4.py

# r0 calibrator
r0_calib = LSTR0Corrections(pedestal_path=pedestal_file, offset=offset_value,
                            r1_sample_start=2, r1_sample_end=38, tel_id=tel_id )

@rlopezcoto
Copy link
Contributor

@FrancaCassol can you change the default values @morcuended found in the visualization scripts? We can merge afterwards

@rlopezcoto rlopezcoto merged commit d14eb9f into cta-observatory:master Jul 1, 2020
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