Update the ADC->fC table, simplify the charge reconstruction algorith… #22974

jkunkle · 2018-04-16T13:16:30Z

Updates to enable the HCAL laser monitoring. With this change, I take the opportunity to make a simplification of the code which performs the laser monitoring charge reconstruction. We have implemented the lasermon system in firmware rather than hardware. This means that we can remove the complicated matching algorithm because the firmware guarantees fixed latency between channels (the hardware implementation did not). The relevant changes are,

RecoMET/METProducers/src/HcalNoiseInfoProducer.cc : Update the adc->fC table to use qie10 ADCs
RecoMET/METProducers/src/HcalNoiseInfoProducer.cc : Update the reconstruction algorithm to simply take the first N samples of each channel where N is a new configurable parameter, laserMonitorSamples_.
RecoMET/METProducers/src/HcalNoiseInfoProducer.cc : Integrate the charge using a simple peak finding algorithm rather than integrating the whole readout
-RecoMET/METFilters/plugins/HcalLaserEventFilter.cc - enable the laser filtering and update the threshold to 5000

The above changes have been tested with a recent global run for consistency against the old method and that the changes propagate to the Met filter

@deguio @mariadalfonso

…m, enable the MET filter

cmsbuild · 2018-04-16T13:16:48Z

The code-checks are being triggered in jenkins.

cmsbuild · 2018-04-16T13:18:35Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22974/4357

Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22974/4357/git-diff.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22974/4357/git-diff.patch | patch -p1

You can run scram build code-checks to apply code checks directly

cmsbuild · 2018-04-16T13:22:43Z

The code-checks are being triggered in jenkins.

cmsbuild · 2018-04-16T13:25:22Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22974/4359

cmsbuild · 2018-04-16T13:25:43Z

A new Pull Request was created by @jkunkle for master.

It involves the following packages:

RecoMET/METFilters
RecoMET/METProducers

@perrotta, @monttj, @cmsbuild, @slava77, @gpetruc, @arizzi can you please review it and eventually sign? Thanks.
@TaiSakuma, @gouskos, @ahinzmann, @mmarionncern, @jdamgov, @jdolen, @nhanvtran, @gkasieczka, @schoef, @mariadalfonso, @seemasharmafnal this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

slava77 · 2018-04-16T13:31:25Z

RecoMET/METProducers/src/HcalNoiseInfoProducer.cc

-                                 7272,7597,7922,8247,8572,8897,9222,9547,10197,10847,11497,12147,
-                                 12797,13447,14097,15072,16047,17022,17997,19297,20597,21897,23522,25147};
+  //adc2fCHF = std::vector<float> {-3,-0.4,2.2,4.8,7.4,10,12.6,15.2,17.8,20.4,23,25.6,28.2,30.8,33.4,
+  //                               36,41.2,46.4,51.6,56.8,62,67.2,73,80.8,88.6,96.4,104,114.4,124.8,135,


is this commented out code needed?
Please remove or add comments inline in the code why the commented out block is relevant

Indeed, I neglected to remove it. I've pushed the change

slava77 · 2018-04-16T13:31:39Z

@cmsbuild please test

cmsbuild · 2018-04-16T13:32:08Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/27508/console Started: 2018/04/16 15:32

cmsbuild · 2018-05-23T14:13:14Z

The code-checks are being triggered in jenkins.

cmsbuild · 2018-05-23T14:17:03Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-22974/4790

cmsbuild · 2018-05-23T14:19:09Z

Pull request #22974 was updated. @perrotta, @monttj, @cmsbuild, @slava77, @gpetruc, @arizzi can you please check and sign again.

jkunkle · 2018-05-23T14:58:09Z

I made the suggested code update. The testing prescription is not working just because the CMSSW version is outdated, I repeat the instructions below with updated CMSSW version

cmsrel CMSSW_10_2_X_2018-05-22-2300
cd CMSSW_10_2_X_2018-05-22-2300/src
cmsenv
git cms-merge-topic 22974
cp -r /afs/cern.ch/user/j/jkunkle//public/HcalLaserCodeTest2/* .
scram b -j4
cmsDriver.py step1 --conditions 101X_dataRun2_Prompt_v9 -s RAW2DIGI,RECO --data --era Run2_2018 --eventcontent RECO,AOD --datatier RECO,AOD --filein root://eoscms.cern.ch//eos/cms/store/data/Commissioning2018/MinimumBias/RAW/v1/000/311/869/00000/5EA73380-B727-E811-9B25-FA163E9BBEF9.root  -n 100   --customise Configuration/DataProcessing/RecoTLR.customisePostEra_Run2_2018 
cmsDriver.py step2 --filein file:step1_RAW2DIGI_RECO.root  --data --eventcontent MINIAOD --runUnscheduled --datatier MINIAOD --conditions 101X_dataRun2_Prompt_v9  --step PAT   --era Run2_2018 -n 100
cmsRun config_test.py

perrotta · 2018-05-23T20:03:38Z

please test

cmsbuild · 2018-05-23T20:06:03Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/28144/console Started: 2018/05/23 22:06

perrotta · 2018-05-23T20:47:17Z

@jkunkle : in your recipe, how can a 10_1_X GT work in 10_2_X ?

cmsbuild · 2018-05-23T22:13:55Z

+1
Tested at: 3ba6b1a
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-22974/28144/summary.html

cmsbuild · 2018-05-23T22:13:58Z

Comparison job queued.

cmsbuild · 2018-05-24T00:07:47Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-22974/28144/summary.html

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 31
DQMHistoTests: Total histograms compared: 2901712
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2901520
DQMHistoTests: Total skipped: 190
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 30 files compared)
Checked 128 log files, 14 edm output root files, 31 DQM output files

perrotta · 2018-05-24T09:04:53Z

Evidently you never tried the recipe in #22974 (comment), which is rather disappointing because I am supposed to have more interesting things to do rather than debugging your private test code.
However, after the following changes in the test script/recipe:

Correct GT for 102X
Fix the "trackeness" of the parameter vetoByLaserMonitor in your config_test.py
Correctly renamed the source file in your config_test.py
I managed to run it and I can get the expected results: that's good!

While running that test, in the reco step I repeatedly get the following warnings in outputs (e.g.):

Begin processing the 3rd record. Run 311869, Event 1469500, LumiSection 72 on stream 0 at 24-May-2018 10:44:11.806 CEST
%MSG-w NoModule:   ByClusterSummaryMultiplicityPairEventFilter:toomanystripclus53X  24-May-2018 10:44:11 CEST Run: 311869 Event: 1469500
No information for requested module 5. Please check in the Provinence Infomation for proper modules.
%MSG
%MSG-w NoModule:   ByClusterSummaryMultiplicityPairEventFilter:toomanystripclus53X  24-May-2018 10:44:11 CEST Run: 311869 Event: 1469500
No information for requested module 0. Please check in the Provinence Infomation for proper modules.
%MSG

They may be originated by the filter code (not necessarily from these latest updates: but you have some related flag in your test, even if the warning itself comes from the Tracker ClusterSummary [1]), and they could become rather annoying if and when it will be switched on. But maybe they just derive from a loose threshold for warnings which was set in your test.
In any case, please have a look and verify whether those warnings are indeed justified, and whether they do point to any fix needed in the filter code or config.

[1] http://cmslxr.fnal.gov/source/DataFormats/TrackerCommon/src/ClusterSummary.cc#0047

jkunkle · 2018-05-24T15:10:50Z

I certainly tried the the testing procedure that I gave, and it produces the expected results as-is. If you observed some failure in the procedure that I gave (which I did not see, since it worked for me) then it would be good to know what that is. Otherwise it is up to you if you want to make unneeded changes to the testing procedure.

I used the GT that was consistent with the conditions used when the data sample was recorded. It is never clear to me what the 'correct' prescription for applying the GT is (if you know of some clear prescription please let me know), but I kept the GT associated to the conditions of that run.

I don't see any need to change the trackedness of the configuration for the test, and I don't see any problem with the source file name, again, it works for me as-is.

The warnings that you see are not related to this PR. This can be checked by simply not merging this pull request and running the cmsDriver commands. My aim was to run the 'nominal' data processing chain by using the cmsDriver commands. Perhaps I'm not running the exact correct cmsDriver command which exposes these warnings.

perrotta · 2018-05-25T09:30:23Z

+1

Enable the HCAL laser monitoring (plus some tuning of the laser MET filter, and some code cleaning)
It potentially affects real data runs with HCAL laser on
No effect is visible on usual MC and data workflows, but the expected behavior had been confirmed while running it on commissioning data taken with the laser firing

fabiocos · 2018-05-26T16:02:23Z

+1

fabiocos · 2018-05-26T16:02:29Z

merge

Update the ADC->fC table, simplify the charge reconstruction algorith…

97b69dd

…m, enable the MET filter

cmsbuild added this to the CMSSW_10_2_X milestone Apr 16, 2018

cmsbuild added analysis-pending code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Apr 16, 2018

cmsbuild added code-checks-rejected and removed code-checks-pending labels Apr 16, 2018

update with code-check results

1003ae7

cmsbuild added code-checks-pending and removed code-checks-rejected labels Apr 16, 2018

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 16, 2018

slava77 reviewed Apr 16, 2018

View reviewed changes

cmsbuild added tests-started and removed tests-pending labels Apr 16, 2018

remove old commented code

b065393

cmsbuild added code-checks-pending and removed code-checks-approved tests-started labels Apr 16, 2018

cmsbuild added code-checks-approved and removed code-checks-pending labels May 23, 2018

cmsbuild added tests-started and removed tests-pending labels May 23, 2018

cmsbuild added tests-approved and removed tests-started labels May 23, 2018

cmsbuild added comparison-available and removed comparison-pending labels May 24, 2018

cmsbuild added reconstruction-approved and removed reconstruction-pending labels May 25, 2018

cmsbuild added orp-approved and removed orp-pending labels May 26, 2018

cmsbuild merged commit 9138de7 into cms-sw:master May 26, 2018

slava77 mentioned this pull request Nov 23, 2018

Flag_hcalLaserEventFilter is broken when running on of pre-10X data in 102X+ releases #25324

Closed

mmusich mentioned this pull request May 6, 2022

Possible misconfiguration of TrackingPOGFilters #37738

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update the ADC->fC table, simplify the charge reconstruction algorith… #22974

Update the ADC->fC table, simplify the charge reconstruction algorith… #22974

jkunkle commented Apr 16, 2018 •

edited

cmsbuild commented Apr 16, 2018

cmsbuild commented Apr 16, 2018

cmsbuild commented Apr 16, 2018

cmsbuild commented Apr 16, 2018

cmsbuild commented Apr 16, 2018

slava77 Apr 16, 2018

jkunkle Apr 16, 2018

slava77 commented Apr 16, 2018

cmsbuild commented Apr 16, 2018 •

edited

cmsbuild commented May 23, 2018

cmsbuild commented May 23, 2018

cmsbuild commented May 23, 2018

jkunkle commented May 23, 2018 •

edited

perrotta commented May 23, 2018

cmsbuild commented May 23, 2018 •

edited

perrotta commented May 23, 2018

cmsbuild commented May 23, 2018

cmsbuild commented May 23, 2018

cmsbuild commented May 24, 2018

perrotta commented May 24, 2018

jkunkle commented May 24, 2018

perrotta commented May 25, 2018

fabiocos commented May 26, 2018

fabiocos commented May 26, 2018

Update the ADC->fC table, simplify the charge reconstruction algorith… #22974

Update the ADC->fC table, simplify the charge reconstruction algorith… #22974

Conversation

jkunkle commented Apr 16, 2018 • edited

cmsbuild commented Apr 16, 2018

cmsbuild commented Apr 16, 2018

cmsbuild commented Apr 16, 2018

cmsbuild commented Apr 16, 2018

cmsbuild commented Apr 16, 2018

slava77 Apr 16, 2018

Choose a reason for hiding this comment

jkunkle Apr 16, 2018

Choose a reason for hiding this comment

slava77 commented Apr 16, 2018

cmsbuild commented Apr 16, 2018 • edited

cmsbuild commented May 23, 2018

cmsbuild commented May 23, 2018

cmsbuild commented May 23, 2018

jkunkle commented May 23, 2018 • edited

perrotta commented May 23, 2018

cmsbuild commented May 23, 2018 • edited

perrotta commented May 23, 2018

cmsbuild commented May 23, 2018

cmsbuild commented May 23, 2018

cmsbuild commented May 24, 2018

perrotta commented May 24, 2018

jkunkle commented May 24, 2018

perrotta commented May 25, 2018

fabiocos commented May 26, 2018

fabiocos commented May 26, 2018

jkunkle commented Apr 16, 2018 •

edited

cmsbuild commented Apr 16, 2018 •

edited

jkunkle commented May 23, 2018 •

edited

cmsbuild commented May 23, 2018 •

edited