Feature docs migration: Round II: Three macros and fitting.fitScanData #123

lmoureaux · 2018-06-29T15:36:45Z

Description

Add the documentation for:

clusterAnaScurve.py
gemTreeDrawWrapper.py
packageFiles4Docker.py
fitting.fitScanData

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Motivation and Context

#94

How Has This Been Tested?

Docs produced and viewed in man and a Web browser. Build available here.

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

Currently only Synopsis and Description

bdorney · 2018-07-03T08:53:03Z

For clusterAnaScurve.py the following options are not reported:

  Options for channel mask decisionsParameters which specify how Dead, Noisy, and High Pedestal Channels are charaterized:
    --maxEffPedPercent=maxEffPedPercent
                        Percentage, Threshold for setting the HighEffPed mask
                        reason, if channel (effPed > maxEffPedPercent * nevts)
                        then HighEffPed is set
    --highNoiseCut=highNoiseCut
                        Threshold for setting the HighNoise maskReason, if
                        channel (scurve_sigma > highNoiseCut) then HighNoise
                        is set
    --deadChanCutLow=deadChanCutLow
                        If channel (deadChanCutLow < scurve_sigma <
                        deadChanCutHigh) then DeadChannel is set
    --deadChanCutHigh=deadChanCutHigh
                        If channel (deadChanCutHigh < scurve_sigma <
                        deadChanCutHigh) then DeadChannel is set

Most likely they did not make their way into the README.md but right now would be a good time to add them.

We should also make some statement that the immediate subdirectories of $DATA_PATH should be expected to be the GEMINImXLY directories which was not readily apparent previously. User reported this was not known which indirectly leads to #124 (the real fix should include some error message in the script of course but putting info into the documentation will also help!).

bdorney · 2018-07-03T08:57:18Z

packageFiles4Docker.rst

I assume this is a typo and refers to packageFiles4Docker.py?

bdorney · 2018-07-03T09:01:55Z

fitting/fitScanData.py

+                self.scanFuncs[vfat][ch] = r.TF1('scurveFit_vfat%i_chan%i'%(vfat,ch),'[3]*TMath::Erf((TMath::Max([2],x)-[0])/(TMath::Sqrt(2)*[1]))+[3]',
+                        self.calDAC2Q_m[vfat]*1+self.calDAC2Q_b[vfat],self.calDAC2Q_m[vfat]*253+self.calDAC2Q_b[vfat])
+                self.scanHistos[vfat][ch] = r.TH1D('scurve_vfat%i_chan%i_h'%(vfat,ch),'scurve_vfat%i_chan%i_h'%(vfat,ch),
+                        254,self.calDAC2Q_m[vfat]*0.5+self.calDAC2Q_b[vfat],self.calDAC2Q_m[vfat]*254.5+self.calDAC2Q_b[vfat])


What is the reason for this change? Which commit is this coming from? In a documentation PR I would not expect the algorithm to be changed

Which commit is this coming from?

For the record: 3991798 I'll revert and update.

bdorney · 2018-07-03T09:04:05Z

fitting/fitScanData.py

@@ -84,25 +170,28 @@ def feed(self, event):
                if(event.vcal > 254):
                    self.scanCount[event.vfatN][event.vfatCH] += event.Nhits
            else:
-                charge = self.calDAC2Q_m[event.vfatN]*(event.vcal)+self.calDAC2Q_b[event.vfatN]
+                charge = self.calDAC2Q_m[event.vfatN]*(256-event.vcal)+self.calDAC2Q_b[event.vfatN]


In a documentation PR we should not change the algorithm.

bdorney · 2018-07-03T09:04:45Z

fitting/fitScanData.py

-        chargeBin = first_index_gt(self.scanHistosChargeBins[event.vfatN][event.vfatCH], charge)-1
-        self.scanHistos[event.vfatN][event.vfatCH].SetBinContent(chargeBin,event.Nhits)
-        self.scanHistos[event.vfatN][event.vfatCH].SetBinError(chargeBin,sqrt(event.Nhits))
+        self.scanHistos[event.vfatN][event.vfatCH].Fill(charge,event.Nhits)


This appears to be incorrect; I think you merged/rebased from a bad starting point.

bdorney · 2018-07-03T09:05:11Z

fitting/fitScanData.py

-                fitTF1 = r.TF1('myERF','[3]*TMath::Erf((TMath::Max([2],x)-[0])/(TMath::Sqrt(2)*[1]))+[3]',
-                            self.calDAC2Q_m[vfat]*1+self.calDAC2Q_b[vfat],self.calDAC2Q_m[vfat]*253+self.calDAC2Q_b[vfat])
+            fitTF1 = r.TF1('myERF','[3]*TMath::Erf((TMath::Max([2],x)-[0])/(TMath::Sqrt(2)*[1]))+[3]',
+                        self.calDAC2Q_m[vfat]*1+self.calDAC2Q_b[vfat],self.calDAC2Q_m[vfat]*253+self.calDAC2Q_b[vfat])


In a documentation PR we should not change the algorithm.

bdorney · 2018-07-03T09:06:08Z

fitting/fitScanData.py

+                    print "| ------ | -- | ------- | ------ | -- | ------- | ------ | -- | ------- |"
+                while(stepN < 15):
+                    rand = max(0.0, random.Gaus(10, 5)) # do not accept negative numbers
+


For sure this is not correct as for the case of vfat3 it will cause only 1/2 of the CAL_DAC register to be scanned. Double check your commit history and revert this; you may need to cherry-pick to remove bad commits or resolve this issue.

bdorney · 2018-07-03T09:06:31Z

fitting/fitScanData.py

+                    fitTF1.SetParameter(0, self.calDAC2Q_m[vfat]*(8+stepN*8)+self.calDAC2Q_b[vfat])
+                    fitTF1.SetParameter(1, self.calDAC2Q_m[vfat]*rand)
+                    fitTF1.SetParameter(2, self.calDAC2Q_m[vfat]*(8+stepN*8)+self.calDAC2Q_b[vfat])
+                    fitTF1.SetParameter(3, self.Nev[vfat][ch]/2.)


Algo shouldn't change in a documentation PR.

bdorney · 2018-07-03T09:06:36Z

fitting/fitScanData.py

+                    fitTF1.SetParLimits(0, 0.01, self.calDAC2Q_m[vfat]*(256)+self.calDAC2Q_b[vfat])
+                    fitTF1.SetParLimits(1, 0.0,  self.calDAC2Q_m[vfat]*(128)+self.calDAC2Q_b[vfat])
+                    fitTF1.SetParLimits(2, 0.0,  self.calDAC2Q_m[vfat]*(256)+self.calDAC2Q_b[vfat])
+                    fitTF1.SetParLimits(3, 0.0,  self.Nev[vfat][ch] * 2.)


bdorney · 2018-07-03T09:06:44Z

fitting/fitScanData.py

+                                    0.0,
+                                    self.calDAC2Q_m[vfat]*(8+stepN*8)+self.calDAC2Q_b[vfat],
+                                    self.calDAC2Q_m[vfat]*(256)+self.calDAC2Q_b[vfat]
+                                )


bdorney · 2018-07-03T09:06:53Z

fitting/fitScanData.py

-                            self.scanFitResults[3][vfat][ch],
-                            self.scanFitResults[5][vfat][ch],
-                            self.scanFitResults[3][vfat][ch] / self.scanFitResults[5][vfat][ch])
-                    pass


bdorney · 2018-07-03T09:07:55Z

mapping/chamberInfo.py

@@ -93,7 +92,7 @@
    #        #Set the Latency - 10x10 PMT on R&D Setup
    #        "CFG_LATENCY":98,
    #        #Correct the bug in the shaper
-    #        "CFG_PT":7,
+    #        "CFG_PT":3,


And this is 873af00

bdorney · 2018-07-03T09:08:01Z

mapping/chamberInfo.py

@@ -128,7 +127,7 @@
    #        #Set the Latency - 10x10 PMT on R&D Setup
    #        "CFG_LATENCY":98,
    #        #Correct the bug in the shaper
-    #        "CFG_PT":7,
+    #        "CFG_PT":3,


bdorney

It seems like the starting point for these changes was not chosen adequately as significant changes to the algorithms are made; no go for a documentation PR. Double check branch/merge/rebase points and revert commits where necessary.

bdorney · 2018-07-03T09:12:07Z

I didn't have a look at the added documentation for the others:

gemTreeDrawWrapper.py
packageFiles4Docker.py
fitting.fitScanData

But my overall comment would be to xcheck the help menu of the actual script to make sure what is included in the documentation reflects that.

Also due to laziness of imports we may have some options that do nothing (since we use a common options menu for several scripts). This is certainly the case in vfatqc. If this is the case here we should try not to have those in the documentation (not sure if that is possible).

lmoureaux · 2018-07-03T09:51:14Z

Probably forgot to refresh the files in my editor after a git rebase. Will fix.

See PR cms-gem-daq-project#123

* Don't duplicate the author list everywhere * Don't hard-code the copyright date * Add a description to the texinfo page

See PR cms-gem-daq-project#123

lmoureaux · 2018-07-03T13:13:10Z

Removed all code changes from history.

lmoureaux · 2018-07-03T13:17:25Z

For clusterAnaScurve.py the following options are not reported:

The documentation says:

Finally clusterAnaScurve.py can also be passed the cut values used in assigning a maskReason described at Providing Cuts for maskReason at Runtime.

I copied this from the README, but adding the options is a 10min change.

bdorney · 2018-07-03T13:31:58Z

I copied this from the README, but adding the options is a 10min change.

Great, so I trust you'll be able to quickly implement it.

--maxEffPedPercent --highNoiseCut --deadChanCutLow --deadChanCutHigh See PR cms-gem-daq-project#123

-s -v See PR cms-gem-daq-project#123

lmoureaux · 2018-07-03T14:13:27Z

But my overall comment would be to xcheck the help menu of the actual script to make sure what is included in the documentation reflects that.

Done.

Also due to laziness of imports we may have some options that do nothing (since we use a common options menu for several scripts). This is certainly the case in vfatqc. If this is the case here we should try not to have those in the documentation (not sure if that is possible).

Easy, just don't include it in the docstring (it will still be shown by -h)

bdorney · 2018-07-04T05:45:28Z

travis had a time out issue again. Could you re-start 156.6?

Probably everything is fine; but should xcheck before merging.

jsturdy

Thanks!

See PR #123

lmoureaux added 6 commits June 29, 2018 14:54

Add man page for gemTreeDrawWrapper.py

a453c9f

Currently only Synopsis and Description

Document arguments of gemTreeDrawWrapper.py

70e2350

Fix copy-paste error

1c6352f

Add examples and env docs for gemTreeDrawWrapper.py

80c9b54

Convert gemTreeDrawWrapper.py's internal docs to rst

b832144

Add man page for packageFiles4Docker

663825c

bdorney reviewed Jul 3, 2018

View reviewed changes

bdorney suggested changes Jul 3, 2018

View reviewed changes

bdorney assigned lmoureaux Jul 3, 2018

bdorney added Status: On Hold Priority: Medium Status: Blocked Status: Revision Needed and removed Priority: Medium labels Jul 3, 2018

lmoureaux added 2 commits July 3, 2018 15:04

Document DeadChannelFinder

15b1295

See PR cms-gem-daq-project#123

Include sphinx.ext.pngmath for maths in docs

5829846

lmoureaux added 5 commits July 3, 2018 15:05

Fix typos in DeadChannelFinder docs

ffb51e6

Document ScanDataFitter and fitScanData

e83c92d

Add a short intro to fitting.rst

2115d05

Update Sphinx configuration file

fc01c1c

* Don't duplicate the author list everywhere * Don't hard-code the copyright date * Add a description to the texinfo page

Add man page for clusterAnaScurve.py

d0f80a4

See PR cms-gem-daq-project#123

lmoureaux force-pushed the feature_docs_migration branch from 873af00 to d0f80a4 Compare July 3, 2018 13:12

Add undocumented options of clusterAnaScurve.py

a7ff025

--maxEffPedPercent --highNoiseCut --deadChanCutLow --deadChanCutHigh See PR cms-gem-daq-project#123

lmoureaux added a commit to lmoureaux/gem-plotting-tools that referenced this pull request Jul 3, 2018

Add undocumented options for gemTreeDrawWrapper.py

b7fb74f

-s -v See PR cms-gem-daq-project#123

lmoureaux force-pushed the feature_docs_migration branch from b7fb74f to a7ff025 Compare July 3, 2018 14:10

bdorney approved these changes Jul 4, 2018

View reviewed changes

bdorney added Status: Review Needed Priority: Medium Type: Documentation and removed Status: Blocked Status: On Hold Status: Revision Needed labels Jul 4, 2018

jsturdy approved these changes Jul 4, 2018

View reviewed changes

lmoureaux mentioned this pull request Jul 4, 2018

Developer instructions #125

Merged

8 tasks

jsturdy merged commit b396b34 into cms-gem-daq-project:develop Jul 4, 2018

jsturdy pushed a commit that referenced this pull request Jul 4, 2018

Document DeadChannelFinder

8a3e679

See PR #123

jsturdy pushed a commit that referenced this pull request Jul 4, 2018

Add man page for clusterAnaScurve.py

6a0580c

See PR #123

lmoureaux deleted the feature_docs_migration branch July 9, 2018 14:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature docs migration: Round II: Three macros and fitting.fitScanData #123

Feature docs migration: Round II: Three macros and fitting.fitScanData #123

lmoureaux commented Jun 29, 2018 •

edited

Loading

bdorney commented Jul 3, 2018 •

edited

Loading

bdorney commented Jul 3, 2018

bdorney Jul 3, 2018 •

edited

Loading

lmoureaux Jul 3, 2018

bdorney Jul 3, 2018

bdorney Jul 3, 2018

bdorney Jul 3, 2018

bdorney Jul 3, 2018

bdorney Jul 3, 2018

bdorney Jul 3, 2018

bdorney Jul 3, 2018

bdorney Jul 3, 2018

bdorney Jul 3, 2018

lmoureaux Jul 3, 2018

bdorney Jul 3, 2018

bdorney left a comment

bdorney commented Jul 3, 2018

lmoureaux commented Jul 3, 2018

lmoureaux commented Jul 3, 2018

lmoureaux commented Jul 3, 2018

bdorney commented Jul 3, 2018

lmoureaux commented Jul 3, 2018

bdorney commented Jul 4, 2018

jsturdy left a comment

Feature docs migration: Round II: Three macros and fitting.fitScanData #123

Feature docs migration: Round II: Three macros and fitting.fitScanData #123

Conversation

lmoureaux commented Jun 29, 2018 • edited Loading

Description

Types of changes

Motivation and Context

How Has This Been Tested?

Checklist:

bdorney commented Jul 3, 2018 • edited Loading

bdorney commented Jul 3, 2018

bdorney Jul 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bdorney left a comment

Choose a reason for hiding this comment

bdorney commented Jul 3, 2018

lmoureaux commented Jul 3, 2018

lmoureaux commented Jul 3, 2018

lmoureaux commented Jul 3, 2018

bdorney commented Jul 3, 2018

lmoureaux commented Jul 3, 2018

bdorney commented Jul 4, 2018

jsturdy left a comment

Choose a reason for hiding this comment

lmoureaux commented Jun 29, 2018 •

edited

Loading

bdorney commented Jul 3, 2018 •

edited

Loading

bdorney Jul 3, 2018 •

edited

Loading