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

Allow the user to set the maxChi2 in the scurve fitting algorithm #258

Closed AndrewLevin closed 4 years ago

AndrewLevin commented 4 years ago

Currently the maxChi2 in the scurve fitting algorithm is hardcoded to 50.

Description

Adds an argument maxChi2 which has a default of 10.

Types of changes

Motivation and Context

This may resolve issue https://github.com/cms-gem-daq-project/gem-plotting-tools/issues/257. However, I cannot reproduce the issue, so I am not sure.

How Has This Been Tested?

Yes, I ran this successfully on the scurve data mentioned in the issue https://github.com/cms-gem-daq-project/gem-plotting-tools/issues/257

Screenshots (if appropriate):

Checklist:

AndrewLevin commented 4 years ago

Here is the scurve sigma distribution for VFAT1 of GE11-X-S-INDIA-0017, the problematic case reported in the issue:

With the HEAD of develop:

scurve_sigmas

With this pull request, with maxChi2 set to 10 (the algorithm involves random seeds, so below is the result of running three times with different random seed initialization):

scurve_sigmas

scurve_sigmas

scurve_sigmas

jsturdy commented 4 years ago

this probably needs a rebase due to #252

AndrewLevin commented 4 years ago

I did rebase it. Is there still a problem?

jsturdy commented 4 years ago

I did rebase it. Is there still a problem?

github shows it as still "out-of-date"... looks like you rebased on a local branch

* dacf5df        (alevin/dac_scan_fit_normalize_chisquared) fix another instance where detName is not updated inside of a loop
* 75bb009        correctly set ohKey and detName inside of loops
* f386745        change chisquared/ndof cut from 20 to 15
* ef1f98d        change cut on chisquared to chisquared/ndof
| * 48d545f      (gemdaq/develop) fix the vfatID array (#263)
| * 134f54f      Get `VREF_ADC` values from the database (#264)
|/  
| * 480565f      (alevin/max_chi2_arg) allow the user to set the maxChi2 in the scurve fitting algorithm
|/  
* 57a8f53        (alevin/develop) [BugFix] Restricted y axis range in SBitThresh Summary Graph (#261)
AndrewLevin commented 4 years ago

ok, I have tried to rebase again, is it good now?