search

cms-gem-daq-project / gem-plotting-tools

Repository for GEM commissioning plotting tools
GNU General Public License v3.0
1 stars 26 forks source link

Apply scurve quality masks in ARM DAC calibration #239

Closed AndrewLevin closed 5 years ago

AndrewLevin commented 5 years ago

Applies quality cuts to the scurve fits that are used in the ARM DAC calibration with configurable cut values.

Description

This pull request makes the following changes:

  1. Fixes an obvious mistake with the upper limit of the pedestal parameter in the scurve fit.
  2. Adds the number of events to the TTree that is output from the scurve analysis. This is needed to apply the quality cut on the effective pedestal.
  3. Moves the default cut values for the 4 mask criteria into anaInfo.py. The default values were taken from https://github.com/cms-gem-daq-project/sw_utils/blob/develop/scripts/calibrateArmDac.sh, not from the default values set are currently set in the argparse argument construction.
  4. Moves the argparse block associated with these cut values into anaoptions.py.
  5. Redesigns the feeding of data into the ARM DAC calibration such that it is based on the TTrees instead of the TGraphErrors that are output from the scurve analysis.
  6. Removes the scurve fits for which any of the four quality mask criteria are satisfied.

Types of changes

Motivation and Context

Intended to resolve issue https://github.com/cms-gem-daq-project/gem-plotting-tools/issues/235

How Has This Been Tested?

Yes, I tested both the updated scurve analysis and the ARM DAC calibration on previously collected ARM DAC calibration data from the detector GE11-X-S-CERN-0010. The "canvScurveMeanVsThrDac_Summary" ARM DAC calibration plots are shown below.

With the head of develop:

canvScurveMeanVsThrDac_Summary

With this pull request:

canvScurveMeanVsThrDac_Summary

There are slight changes to the plots e.g. VFATs 5, 6, and 10. I would conclude that these changes are improvements, given that Gaussian fits to the scurveMean distribution that previously failed are now succeeding, and some of large error bars are reduced.

Checklist:

bdorney commented 5 years ago

So in addition to the comments I've put above I've listed this as "blocked" and "on hold" @jsturdy has requested that PR's should be first discussed in issues and algorithm's outlined before submitting a PR. There have been several changes here that where not originally outlined in the issue.

Before making the PR it would have been good to see a thorough comparison of these plots (at the very least on the same TPad) not to mention something more indepth (e.g. how do pull's/residuals between data & fit behave) and then some plan to attempt to compare trimming before/after this change.

AndrewLevin commented 5 years ago

I found that in two cases (vfat1 and vfat2 in slide 8 of https://indico.cern.ch/event/837005/contributions/3509259/attachments/1884624/3106257/ALevin_calibrateThrDac_SWDevMeeting_23Jul2019.pdf), after applying the channel masks, a Gaussian fit was performed on a TGraph which was constructed from a single channel, which resulted in an unphysical negative Gaussian sigma. These points were already excluded from the fit, but they were still shown as non-zero points on the plot. Now, I have added protection against this by requiring that at least 10 channels are unmasked, otherwise the Gaussian fit is not attempted. The effect of this change can be seen below for the same example as in the slides (GE11-X-S-PAK-0007):

Before:

canvScurveMeanVsThrDac_Summary

After:

canvScurveMeanVsThrDac_Summary

bdorney commented 5 years ago

Looks like this needs a rebase.

Please let us know when this is done and it's ready for re-review.

AndrewLevin commented 5 years ago

It is ready for re-review. I have uploaded some slides for tomorrow's meeting: https://indico.cern.ch/event/838248/contributions/3515801/attachments/1887448/3111845/ALevin_calibrateThrDac_SWDevMeeting_30Jul2019.pdf

AndrewLevin commented 5 years ago

All of your requested changes have been implemented.

bdorney commented 5 years ago

So following feedback from SW Dev meeting we agreed (@jsturdy, myself and @AndrewLevin) to:

AndrewLevin commented 5 years ago

I have made those changes in the last two commits.

TGraphs with zeroed points removed:

canvScurveMeanVsThrDac_Summary

Bad channel summary information printout:

| vfatN | armDacVal | Dead Chan | High Noise | High Eff Ped | Fit At Init Val |
| :---: | :-------: | :-------: | :--------: | :----------: | :-------------: |
| -1 | 17 | 48 | 1671 | 2935 | 48 |
| -1 | 20 | 47 | 1974 | 2934 | 47 |
| -1 | 23 | 48 | 2211 | 2936 | 48 |
| -1 | 25 | 46 | 2272 | 2927 | 46 |
| -1 | 30 | 49 | 596 | 1563 | 49 |
| -1 | 40 | 48 | 349 | 803 | 48 |
| -1 | 50 | 49 | 308 | 481 | 49 |
| -1 | 60 | 48 | 282 | 360 | 48 |
| -1 | 70 | 49 | 254 | 271 | 49 |
| -1 | 80 | 48 | 240 | 161 | 48 |
| -1 | 100 | 50 | 222 | 72 | 50 |
| -1 | 125 | 49 | 228 | 55 | 49 |
| -1 | 150 | 49 | 196 | 49 | 49 |
| -1 | 175 | 77 | 210 | 77 | 77 |
| 0 | 17 | 0 | 57 | 128 | 0 |
| 0 | 20 | 0 | 80 | 128 | 0 |
| 0 | 23 | 0 | 76 | 128 | 0 |
| 0 | 25 | 0 | 72 | 128 | 0 |
| 0 | 30 | 0 | 3 | 38 | 0 |
| 0 | 40 | 0 | 1 | 15 | 0 |
| 0 | 50 | 0 | 1 | 7 | 0 |
| 0 | 60 | 0 | 1 | 7 | 0 |
| 0 | 70 | 0 | 1 | 7 | 0 |
| 0 | 80 | 0 | 1 | 2 | 0 |
| 0 | 100 | 0 | 1 | 0 | 0 |
| 0 | 125 | 0 | 1 | 0 | 0 |
| 0 | 150 | 0 | 1 | 0 | 0 |
| 0 | 175 | 0 | 1 | 0 | 0 |
| 1 | 17 | 0 | 45 | 128 | 0 |
| 1 | 20 | 0 | 52 | 128 | 0 |
| 1 | 23 | 0 | 54 | 128 | 0 |
| 1 | 25 | 0 | 58 | 128 | 0 |
| 1 | 30 | 0 | 1 | 36 | 0 |
| 1 | 40 | 0 | 1 | 9 | 0 |
| 1 | 50 | 0 | 1 | 7 | 0 |
| 1 | 60 | 0 | 1 | 9 | 0 |
| 1 | 70 | 0 | 1 | 8 | 0 |
| 1 | 80 | 0 | 1 | 0 | 0 |
| 1 | 100 | 0 | 1 | 0 | 0 |
| 1 | 125 | 0 | 1 | 0 | 0 |
| 1 | 150 | 0 | 1 | 0 | 0 |
| 1 | 175 | 0 | 1 | 0 | 0 |
...
jsturdy commented 5 years ago

I don't understand the table; I expect one entry per VFAT (ah, one per VFAT per DAC point), no VFATs indexed as -1, and at most a value of 128 in each of the columns. If more than one link is possible by this tool, then as many additional fields as necessary to make the results unique (shelf/AMC/link)

AndrewLevin commented 5 years ago

The tool handles a single detector at a time.

AndrewLevin commented 5 years ago

vfatN = -1 is collective information for all VFATs

bdorney commented 5 years ago

vfatN = -1 is collective information for all VFATs

For this case instead of reporting -1 I would report All in that column in this case.

AndrewLevin commented 5 years ago

Done:

| vfatN | armDacVal | Dead Chan | High Noise | High Eff Ped | Fit At Init Val |
| :---: | :-------: | :-------: | :--------: | :----------: | :-------------: |
| All | 17 | 48 | 1671 | 2935 | 48 |
| All | 20 | 47 | 1974 | 2934 | 47 |
| All | 23 | 48 | 2211 | 2936 | 48 |
| All | 25 | 46 | 2272 | 2927 | 46 |
| All | 30 | 49 | 596 | 1563 | 49 |
| All | 40 | 48 | 349 | 803 | 48 |
| All | 50 | 49 | 308 | 481 | 49 |
| All | 60 | 48 | 282 | 360 | 48 |
| All | 70 | 49 | 254 | 271 | 49 |
| All | 80 | 48 | 240 | 161 | 48 |
| All | 100 | 50 | 222 | 72 | 50 |
| All | 125 | 49 | 228 | 55 | 49 |
| All | 150 | 49 | 196 | 49 | 49 |
| All | 175 | 77 | 210 | 77 | 77 |
| 0 | 17 | 0 | 57 | 128 | 0 |
| 0 | 20 | 0 | 80 | 128 | 0 |
| 0 | 23 | 0 | 76 | 128 | 0 |
| 0 | 25 | 0 | 72 | 128 | 0 |
| 0 | 30 | 0 | 3 | 38 | 0 |
| 0 | 40 | 0 | 1 | 15 | 0 |
| 0 | 50 | 0 | 1 | 7 | 0 |
| 0 | 60 | 0 | 1 | 7 | 0 |
| 0 | 70 | 0 | 1 | 7 | 0 |
| 0 | 80 | 0 | 1 | 2 | 0 |
| 0 | 100 | 0 | 1 | 0 | 0 |
| 0 | 125 | 0 | 1 | 0 | 0 |
| 0 | 150 | 0 | 1 | 0 | 0 |
| 0 | 175 | 0 | 1 | 0 | 0 |
| 1 | 17 | 0 | 45 | 128 | 0 |
| 1 | 20 | 0 | 52 | 128 | 0 |
| 1 | 23 | 0 | 54 | 128 | 0 |
| 1 | 25 | 0 | 58 | 128 | 0 |
| 1 | 30 | 0 | 1 | 36 | 0 |
| 1 | 40 | 0 | 1 | 9 | 0 |
| 1 | 50 | 0 | 1 | 7 | 0 |
| 1 | 60 | 0 | 1 | 9 | 0 |
| 1 | 70 | 0 | 1 | 8 | 0 |
| 1 | 80 | 0 | 1 | 0 | 0 |
| 1 | 100 | 0 | 1 | 0 | 0 |
| 1 | 125 | 0 | 1 | 0 | 0 |
| 1 | 150 | 0 | 1 | 0 | 0 |
| 1 | 175 | 0 | 1 | 0 | 0 |
...