Skip to content
This repository has been archived by the owner on Jan 31, 2022. It is now read-only.

Bug Report: anaUltraScurve.py still has event count hard coded in one spot #37

Closed
1 of 2 tasks
bdorney opened this issue Sep 14, 2017 · 2 comments
Closed
1 of 2 tasks

Comments

@bdorney
Copy link
Contributor

bdorney commented Sep 14, 2017

Brief summary of issue

In anaUltraScurve.py the value for events/2 is still harded at line 151. Here it is set as 500 but should be an input parameter from the input TTree.

This should be changed since if a user takes an scurve with non-default settings (nevt=1000) this part of the code may not work as intended.

Low priority since this will work on scans taken with default parameters.

Types of issue

  • Bug report (report an issue with the code)
  • Feature request (request for change which adds functionality)

Expected Behavior

Value for events/2 at line 151 should not be hard coded but instead be an input parameter.

Current Behavior

Value is hard coded at events/2 = 500 at line 151.

Possible Solution (for bugs)

Change Line 151 to take an input instead of a hard coded value.

Your Environment

  • Version used: 164fcef
  • Shell used: /bin/bash
bdorney pushed a commit to bdorney/gem-plotting-tools that referenced this issue Sep 14, 2017
@bdorney bdorney mentioned this issue Sep 14, 2017
8 tasks
@cbravo135
Copy link
Contributor

I do think this is the wrong way to interpret this bug. We should change the scurve object to be a TGraphAsymmErrors made by dividing 2 histograms so C-P confidence intervals are used. Then all fits will be done with an efficiency of 0 to 1 and have the proper error bars. This would also mean several scurves could be statistically summed together and the analysis would be robust. It would also allow the number of pulses per Vcal bin to be unequal and the fit could still work.

bdorney added a commit that referenced this issue Sep 19, 2017
@bdorney
Copy link
Contributor Author

bdorney commented Sep 19, 2017

I think this is a good suggestion, but I think this is enough of a change where it should be it's own separate issue. For now the proposed solution will serve as a "short term" fix.

@bdorney bdorney closed this as completed Sep 19, 2017
jsturdy pushed a commit to jsturdy/gem-plotting-tools that referenced this issue Mar 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants