Fixing Offline DQM Bug #12103

vkhristenko · 2015-10-26T20:03:20Z

Fixing the bug reported here
https://hypernews.cern.ch/HyperNews/CMS/get/recoDevelopment/1396/1/3/1/2/1/1.html

This is only for Offline DQM!

VK

cmsbuild · 2015-10-26T20:06:10Z

A new Pull Request was created by @vkhristenko (Viktor Khristenko) for CMSSW_7_4_X.

Fixing Offline DQM Bug

It involves the following packages:

DQM/HcalMonitorTasks

@cmsbuild, @danduggan, @vanbesien, @deguio can you please review it and eventually sign? Thanks.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

slava77 · 2015-10-26T20:25:29Z

DQM/HcalMonitorTasks/src/HcalDigiMonitor.cc

@@ -570,6 +570,8 @@ void HcalDigiMonitor::analyze(edm::Event const&e, edm::EventSetup const&s)
 		  i<=FEDNumbering::MAXHCALuTCAFEDID; i++) {
 	  if (i>FEDNumbering::MAXHCALFEDID && i<FEDNumbering::MINHCALuTCAFEDID)
 		  continue;
+	  if (i<1118)


a comment here may be helpful (e.g. skip uTCA non-HF FEDs ).

this hardcoded choice is not good for maintenance: at some point the HBHE uTCA will need to be monitored as well.

also, shouldn't this be if (i > MAXHCALFEDID && i < 1118 )
if I understand correctly, now only HF uTCA FEDs will pass.

slava77 · 2015-10-26T20:27:04Z

@vkhristenko
What is the meaning of This is only for Offline DQM! ?
@danduggan @deguio are you in control of what goes in online DQM in a separate release builds?

slava77 · 2015-10-26T20:27:14Z

@cmsbuild please test

cmsbuild · 2015-10-26T20:31:28Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/9207/console
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/9212/console

vkhristenko · 2015-10-26T20:58:37Z

Hi Slava, so the situation is the following:
I'm currently rewritting HCAL DQM (Online + Offline) from scratch. As far as I've checked, and I might be mistaken, but I believe/hope I'm not, that is all I have to worry about (in terms of packages), Validation seems to use their own modules - therefore only Offline + Online.
Old modules will be completely depreciated starting December 14th(beginning of the Long Technical Stop). Physics Datataking stops at that point and I believe that will be the most optimal time to introduce the updates
if I commit and push (deploy) everything now I have to provide instructions + it will be completely confusing for the shifters/etc... and difficult for me. I do plan start committing newest development quite soon - ~middle of November.
-> Why only for Offline - this question has been extensively discussed with Central DQM - we use something on top that has been running for several month now... both Dan and Federico are aware of this fact.

-> Regarding why no comments - module is to be replaced as mentioned above.

-> Regarding including my newly introduced if statement into the one just above - would of course be better - yes, the point is to skip uTCA/AMC13-based uHBHE FEDs - no explanation. I just did it like that...
This issue first showed up in Online DQM and was fixed earlier today the same way and runs successfully now...

I hope that answers all the questions
VK

cmsbuild · 2015-10-26T22:42:13Z

+1
Tested at: 2390933
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12103/9207/summary.html

cmsbuild · 2015-10-27T09:38:37Z

+1
Tested at: 2390933
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12103/9212/summary.html

cmsbuild · 2015-10-27T10:33:02Z

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

danduggan · 2015-10-27T12:14:32Z

@vkhristenko, I think @slava77's comments should be easy to add, could you please do so, then we can test + approve.

slava77 · 2015-10-27T12:25:34Z

@cmsbuild please test

cmsbuild · 2015-10-27T12:26:37Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/9228/console

slava77 · 2015-10-27T13:10:31Z

@abdoulline

vkhristenko · 2015-10-27T13:41:31Z

I don't know why this change causes this kind of behavior, in particular for this plot.
The part that is changed has to do with extracting number of time samples from raw - that is how it was done for vme. And nothing with occupancies....

The changes that Slava asked were actually quite neccessary.
I'm clueless at this point

VK

slava77 · 2015-10-27T14:10:09Z

@vkhristenko just in case: the plots I posted were with the first version of this PR (only changes in vkhristenko@2390933)
the tests with the latest version are not done yet.

vkhristenko · 2015-10-27T14:13:10Z

@slava77, Yeah yeah, I understood the timeline.

abdoulline · 2015-10-27T14:31:02Z

Well, I'm guseesing what all this might mean...

(1)
In the Unpacker itself Ted Laird has added (March 2015) HF uTCA FEDSs explicitly

http://cmslxr.fnal.gov/source/EventFilter/HcalRawToDigi/plugins/HcalRawToDigi.cc#0033
because FEDs iteration range doesn't include uTCA HF FEDs.

(2) here, in the initial 2390933 DQM change
2390933

                             731   - 1100    (non-HCAL range)

if (i>FEDNumbering::MAXHCALFEDID && i<FEDNumbering::MINHCALuTCAFEDID)
continue;
if (i<1118) // means drop all non-HF uTCA FEDs ?
continue;

(3) emap ( HcalElectronicsMap_v7.05_hlt / HcalElectronicsMap_v7.05_offline or earlier versions)
should all be Run1 -compatible, unless GT is some kind of "frozen" (doesn't have appropriate Run1 IOVs)

vkhristenko · 2015-10-27T14:35:41Z

@abdoulline,
Regarding uTCA HBHE, they are 1100 up 1116.
HF uTCA are 1118, 1120, 1122
Initial change isn't perfect - yes, skips all the non-HF-uTCA FEDs - fixed now.
However, that piece only affects getting number of time samples from raw. it doesn't look at anything else... it only works for vme....

Regarding the unpacker - I don't know....

VK

abdoulline · 2015-10-27T14:45:19Z

OK, thank you for providing HBHE uTCA FEDs (HF ones 1118/1120/1122 is
exactly what's hardcoded in the "patch" of Ted Laird in the unpacker).

If you say:
"However, that piece only affects getting number of time samples from raw.
it doesn't look at anything else... it only works for vme...."

I'm confused by how it can affect Digi occupancy as reported by Slava.
I don't remeber any issues reported about Run 1 occupancies plots...

S.

On Tue, 27 Oct 2015, Viktor Khristenko wrote:

@abdoulline,
Regarding uTCA HBHE, they are 1100 up 1116.
HF uTCA are 1118, 1120, 1122
Initial change isn't perfect - yes, skips all the non-HF-uTCA FEDs - fixed now.
However, that piece only affects getting number of time samples from raw. it doesn't look at anything else... it only
works for vme....

Regarding the unpacker - I don't know....

VK

—
Reply to this email directly or view it on GitHub.[AEx02gjPK9TjJjYEzf0vAyQWB4PofrTvks5o_4NBgaJpZM4GV9K9.gif]

abdoulline · 2015-10-27T15:53:31Z

I've dumped/expanded config step3_RAW2DIGI_L1Reco_RECO_EI_PAT_ALCA_DQM.py
from
runTheMatrix.py -l 4.53 -j 0

and can see that GT 'auto:run1_data_Fake' stands for 74X_dataRun1_v3, which in turn includes HcalElectronicsMap_v7.05_offline with a (first) Run 1 IOV available...
https://cms-conddb.cern.ch/browser/#list/Prod/tags/HcalElectronicsMap_v7.05_offline
And I've dumped its payload and can see it does correspond to Run 1 emap (all-VME HF).
So emap itself should be OK for Run 1...

cmsbuild · 2015-10-27T16:16:37Z

+1
Tested at: 692603e
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12103/9228/summary.html

danduggan · 2015-10-27T16:18:51Z

@abdoulline and @vkhristenko, we just discussed this in the ORP, and if you're happy with the current implementation, then I'll sign off.

vkhristenko · 2015-10-27T16:33:29Z

I would like to wait for comparison of the tests, which is running now...

When it's done, Dan, unless Salavat has objections, I think that we can go ahead.

VK

danduggan · 2015-10-27T16:35:19Z

@vkhristenko perfect.

abdoulline · 2015-10-27T17:05:50Z

Looking at the (two) lines of code update, I have no objections.

On Tue, 27 Oct 2015, Viktor Khristenko wrote:

I would like to wait for comparison of the tests, which is running now...

When it's done, Dan, unless Salavat has objections, I think that we can go ahead.

VK

—
Reply to this email directly or view it on
GitHub.[AEx02rcuEn-wljeZmKnkrxJIA4j9vrPZks5o_57cgaJpZM4GV9K9.gif]

cmsbuild · 2015-10-27T17:38:18Z

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

vkhristenko · 2015-10-27T17:47:08Z

I gotta say that after looking at the comparison (in particular since the newest comparison gives really good match for what didn't match after the initial commit), I gave it a closer look.
There are 2 more variables that do get affected in that part and get used later in the code - Therefore, my apology for that - I was too quick.

However, now it seems to be fine. I think we are ready (from Hcal side)
Just to add that Slava's comment initially regarding the if-statement was correct - again my fault...

VK

danduggan · 2015-10-27T18:13:36Z

+1

cmsbuild · 2015-10-27T18:16:31Z

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_4_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_7_6_X is complete. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar

davidlange6 · 2015-10-28T06:49:12Z

+1

Fixing Offline DQM Bug

Making sure uHBHE FEDs aren't processed in HcalDigiMonitor

2390933

cmsbuild added this to the Next CMSSW_7_4_X milestone Oct 26, 2015

cmsbuild added dqm-pending pending-signatures tests-pending orp-pending comparison-pending labels Oct 26, 2015

slava77 reviewed Oct 26, 2015
View reviewed changes

cmsbuild added tests-started and removed tests-pending labels Oct 26, 2015

cmsbuild added tests-approved and removed tests-started labels Oct 26, 2015

cmsbuild added comparison-available and removed comparison-pending labels Oct 27, 2015

Comments, etc

692603e

cmsbuild added tests-pending comparison-pending and removed comparison-available tests-approved labels Oct 27, 2015

cmsbuild added tests-approved and removed tests-started labels Oct 27, 2015

cmsbuild added comparison-available and removed comparison-pending labels Oct 27, 2015

cmsbuild added dqm-approved fully-signed and removed dqm-pending pending-signatures labels Oct 27, 2015

cmsbuild added orp-approved and removed orp-pending labels Oct 28, 2015

cmsbuild added a commit that referenced this pull request Oct 28, 2015

Merge pull request #12103 from vkhristenko/CMSSW_7_4_X

4fc83fe

Fixing Offline DQM Bug

cmsbuild merged commit 4fc83fe into cms-sw:CMSSW_7_4_X Oct 28, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixing Offline DQM Bug #12103

Fixing Offline DQM Bug #12103

vkhristenko commented Oct 26, 2015

cmsbuild commented Oct 26, 2015

slava77 Oct 26, 2015

slava77 Oct 26, 2015

slava77 commented Oct 26, 2015

slava77 commented Oct 26, 2015

cmsbuild commented Oct 26, 2015

vkhristenko commented Oct 26, 2015

cmsbuild commented Oct 26, 2015

cmsbuild commented Oct 27, 2015

cmsbuild commented Oct 27, 2015

danduggan commented Oct 27, 2015

slava77 commented Oct 27, 2015

cmsbuild commented Oct 27, 2015

slava77 commented Oct 27, 2015

vkhristenko commented Oct 27, 2015

slava77 commented Oct 27, 2015

vkhristenko commented Oct 27, 2015

abdoulline commented Oct 27, 2015

vkhristenko commented Oct 27, 2015

abdoulline commented Oct 27, 2015

abdoulline commented Oct 27, 2015

cmsbuild commented Oct 27, 2015

danduggan commented Oct 27, 2015

vkhristenko commented Oct 27, 2015

danduggan commented Oct 27, 2015

abdoulline commented Oct 27, 2015

cmsbuild commented Oct 27, 2015

vkhristenko commented Oct 27, 2015

danduggan commented Oct 27, 2015

cmsbuild commented Oct 27, 2015

davidlange6 commented Oct 28, 2015

Fixing Offline DQM Bug #12103

Fixing Offline DQM Bug #12103

Conversation

vkhristenko commented Oct 26, 2015

cmsbuild commented Oct 26, 2015

slava77 Oct 26, 2015

Choose a reason for hiding this comment

slava77 Oct 26, 2015

Choose a reason for hiding this comment

slava77 commented Oct 26, 2015

slava77 commented Oct 26, 2015

cmsbuild commented Oct 26, 2015

vkhristenko commented Oct 26, 2015

cmsbuild commented Oct 26, 2015

cmsbuild commented Oct 27, 2015

cmsbuild commented Oct 27, 2015

danduggan commented Oct 27, 2015

slava77 commented Oct 27, 2015

cmsbuild commented Oct 27, 2015

slava77 commented Oct 27, 2015

vkhristenko commented Oct 27, 2015

slava77 commented Oct 27, 2015

vkhristenko commented Oct 27, 2015

abdoulline commented Oct 27, 2015

vkhristenko commented Oct 27, 2015

abdoulline commented Oct 27, 2015

abdoulline commented Oct 27, 2015

cmsbuild commented Oct 27, 2015

danduggan commented Oct 27, 2015

vkhristenko commented Oct 27, 2015

danduggan commented Oct 27, 2015

abdoulline commented Oct 27, 2015

cmsbuild commented Oct 27, 2015

vkhristenko commented Oct 27, 2015

danduggan commented Oct 27, 2015

cmsbuild commented Oct 27, 2015

davidlange6 commented Oct 28, 2015