Fix or suppress warnings related to Phase0 HE in 2017 #17335

kpedro88 · 2017-01-31T13:58:19Z

Followup to #17198.

Validation warnings related to improper handling of TestNumbering for HCAL SimHits have been fixed.
@bsunanda, @hatakeyamak, @kencall at some point we should tweak HcalHitRelabeller to be useful as a general helper class for TestNumbering SimHit processing. There's no reason for the Validation code to use HcalTestNumbering or HcalDDDRecConstants classes unless the actual layer information is needed.

I couldn't figure out the source of the DQM printout (which was using std::cout), so I just commented it out. @deguio @vkhristenko please investigate when you have time. There must be an inconsistency with the constants or something.

cmsbuild · 2017-01-31T13:58:36Z

A new Pull Request was created by @kpedro88 (Kevin Pedro) for CMSSW_9_0_X.

It involves the following packages:

DQMServices/Core
Validation/GlobalHits
Validation/HcalDigis

@cmsbuild, @dmitrijus, @vanbesien, @davidlange6 can you please review it and eventually sign? Thanks.
@barvic this is something you requested to watch as well.
@davidlange6, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

kpedro88 · 2017-01-31T13:59:07Z

please test

cmsbuild · 2017-01-31T13:59:24Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/17531/console Started: 2017/01/31 15:22

davidlange6 · 2017-01-31T14:15:41Z

I don't think removing a error at the level of MonitorElement.cc is the way to proceed here. Its probably better to live with those (a bunch at the start of jobs) while the hcal fixes things properly.

kpedro88 · 2017-01-31T14:16:29Z

would you prefer for it to use LogWarning?

davidlange6 · 2017-01-31T14:26:30Z

it should use the message logger yes - but if it indicates a real problem (which referencing a nonexistent histogram bin likely is), then the fix is rather at the histogram creator level not to flush the message down some dev/null drain.. maybe you can ask the MonitorElement for the histogram title as a hint for further debugging

…

On Jan 31, 2017, at 3:16 PM, Kevin Pedro ***@***.***> wrote: would you prefer for it to use LogWarning? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

davidlange6 · 2017-01-31T14:27:25Z

ah, but the printout already gives you the full name…

…

On Jan 31, 2017, at 3:26 PM, David Lange ***@***.***> wrote: it should use the message logger yes - but if it indicates a real problem (which referencing a nonexistent histogram bin likely is), then the fix is rather at the histogram creator level not to flush the message down some dev/null drain.. maybe you can ask the MonitorElement for the histogram title as a hint for further debugging > On Jan 31, 2017, at 3:16 PM, Kevin Pedro ***@***.***> wrote: > > would you prefer for it to use LogWarning? > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub, or mute the thread. >

kpedro88 · 2017-01-31T14:28:40Z

Yes, unfortunately the DQM code is so impenetrable that I can't figure out where the "real" number of bins is actually determined. I'll update the PR to make it use LogWarning.

cmsbuild · 2017-01-31T14:34:32Z

Pull request #17335 was updated. @cmsbuild, @dmitrijus, @vanbesien, @davidlange6 can you please check and sign again.

kpedro88 · 2017-01-31T14:35:03Z

please test

cmsbuild · 2017-01-31T14:35:15Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/17533/console Started: 2017/01/31 15:35

dmitrijus · 2017-01-31T16:21:54Z

I agree with David, this is something to be fixed in "user" / histogram creator" level. Maybe throw an exception? This way it will be pretty clear who uses the faulty bin....

std::cout was used because this code not only runs in cms framework but also outside of it, e.g. DQMGUI.
And MessageLogger fails quite spectacularly when the service is not available...

I will ping Marco. Perhaps we should/could use:

#if !WITHOUT_CMS_FRAMEWORK
edm::LogWarning(....)
#endif

rovere · 2017-01-31T16:33:11Z

Yes I confirm this will likely fail outside CMSSW: I'd suggest to leave the cout as is or put it inside ifdefs.
I'm not sure what exactly we are talking about, but any error spotted by that cout must be fixed ab origine by the maintainer of the offending code.

kpedro88 · 2017-01-31T16:39:49Z

okay, I've just removed all the changes to DQM code for simplicity. At some point it would be nice to avoid couts where possible.

cmsbuild · 2017-01-31T16:57:21Z

+1
Tested at: af175b1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-17335/17533/summary.html

cmsbuild · 2017-01-31T16:57:24Z

Comparison job queued.

cmsbuild · 2017-01-31T18:22:29Z

+1
Tested at: 74ad19e
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-17335/17537/summary.html

cmsbuild · 2017-01-31T18:22:32Z

Comparison job queued.

cmsbuild · 2017-01-31T19:28:37Z

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

@slava77 comparisons for the following workflows were not done due to missing matrix map:

10824.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2018Design_GenSimFull+DigiFull_2018Design+RecoFull_2018Design+HARVESTFull_2018Design
20034.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D7_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D7+RecoFullGlobal_2023D7+HARVESTFullGlobal_2023D7
21234.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D4_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D4+RecoFullGlobal_2023D4+HARVESTFullGlobal_2023D4
23234.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D8_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D8+RecoFullGlobal_2023D8+HARVESTFullGlobal_2023D8

kpedro88 · 2017-01-31T21:46:09Z

@davidlange6 i guess these warnings will still be in pre3?

davidlange6 · 2017-01-31T21:48:38Z

i just realized I missed it.. yes

slava77 · 2017-02-06T18:18:13Z

@kpedro88
I see that run-2 (2016 data and 2017 MC) workflows have quite a few

*** MonitorElement: WARNING:setBinLabel: attempting to set label of non-existent bin number for ME: Hcal/DigiTask/TimingCut/FED/

in recent tests (seen in 02-06-1100)
wasn't this PR supposed to clean them up?

https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-17423/17630/runTheMatrix-results/136.731_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT_skimSinglePh_HIPM+HARVESTDR2/step3_RunSinglePh2016B+RunSinglePh2016B+HLTDR2_2016+RECODR2_2016reHLT_skimSinglePh_HIPM+HARVESTDR2.log

kpedro88 · 2017-02-06T18:45:39Z

I could only clean up the Validation related issues. HCAL DQM experts are supposed to address the other warnings. @deguio, @vkhristenko

vkhristenko · 2017-02-06T19:13:20Z

I suggest we exclude HCAL DQM from Relvals, HCAL Validation is enough for that
2016 Data - can not be possible, it worked perfectly fine in 2016

VK

deguio · 2017-02-07T09:37:37Z

@kpedro88
yes, the problem must be an inconsistency with the constants as you say. they are being reviewed anyway in preparation for 2017 data taking. I am also fine with removing the DQM from relvals for now. we can include it again once things are more stable.

@DryRun

davidlange6 · 2017-02-17T20:09:03Z

hi all we need a PR for this “solution” by Monday- Thanks

…

On Feb 7, 2017, at 10:37 AM, Federico De Guio ***@***.***> wrote: @kpedro88 yes, the problem must be an inconsistency with the constants as you say. they are being reviewed anyway in preparation for 2017 data taking. I am also fine with removing the DQM from relvals for now. we can include it again once things are more stable. @DryRun — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or mute the thread.

kpedro88 · 2017-02-17T20:10:57Z

s/all/@deguio, @vkhristenko /

vkhristenko · 2017-02-17T20:19:43Z

remove HCAL DQM from Relvals, in HCAL, HCAL Validation is responsible for Release Validation, not HCAL DQM - noone uses it or will use it in the future for relvals.
Therefore, to limit the amount of work we have to do - I want to exclude HCAL DQM from RelVals.

If you know which config is responsible for including hcal dqm sequence into the path - please let me know!

VK

hatakeyamak · 2017-02-17T20:26:34Z

I think we agreed on that (removing DQM, keeping validation. Don't remove DQMOffline/HCAL. We use that for Validation as well as all Validation/xxx stuff). I think just have to find somebody to make this change which is a simple change in config.

dmitrijus · 2017-02-20T14:24:35Z

Ping!

One of the offending modules is "digiPhase1Task", included in,
https://github.com/cms-sw/cmssw/blob/53327033fb356b2a704464439ce79fa47a9d1678/DQM/HcalTasks/python/OfflineSourceSequence_pp.py
And then by https://github.com/cms-sw/cmssw/blob/27ddd5987103f9df1dd40e82129c91f0031e226f/DQMOffline/Configuration/python/DQMOffline_cff.py

What I will is convert these warnings to LogInfo...

davidlange6 · 2017-02-20T14:32:49Z

would rather not run broken stuff..

…

On Feb 20, 2017, at 3:24 PM, Dmitrijus ***@***.***> wrote: Ping! One of the offending modules is "digiPhase1Task", included in, https://github.com/cms-sw/cmssw/blob/53327033fb356b2a704464439ce79fa47a9d1678/DQM/HcalTasks/python/OfflineSourceSequence_pp.py And then by https://github.com/cms-sw/cmssw/blob/27ddd5987103f9df1dd40e82129c91f0031e226f/DQMOffline/Configuration/python/DQMOffline_cff.py What I will is convert these warnings to LogInfo... — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or mute the thread.

davidlange6 · 2017-02-20T17:01:32Z

my solution based on the comments here is in #17578 (It seems we are still converging on the validation of pre4 so some time for a better one)

vkhristenko · 2017-02-20T17:01:50Z

so, I have to say I overlooked a bit here and a previous PR hcaldqm-related:

this PR breaks the code

#17015

using Mapper::getHash;
virtual uint32_t getHash(HcalElectronicsId const& eid) const {...}

and the reason is that the base class specifies the same function without const specifier
give me a few hours to submit a PR

VK

kpedro88 · 2017-02-20T17:03:47Z

@vkhristenko thanks for noticing the bug - but I didn't edit DQM code in this PR...

vkhristenko · 2017-02-20T17:04:35Z

not in this - I was referring to this guy, #17015

Apology Kevin!

VK

vkhristenko · 2017-02-20T17:35:49Z

#17579
VK

kpedro88 added 2 commits January 31, 2017 07:44

handle testnumbering in GlobalHits validation

899cdfe

fix testnumbering handling in HcalDigisValidation

74ad19e

cmsbuild added this to the Next CMSSW_9_0_X milestone Jan 31, 2017

cmsbuild added comparison-pending dqm-pending orp-pending pending-signatures tests-pending labels Jan 31, 2017

cmsbuild added tests-started and removed tests-pending labels Jan 31, 2017

kpedro88 force-pushed the Phase1-HE15 branch from 63720b1 to af175b1 Compare January 31, 2017 14:34

cmsbuild added tests-pending and removed tests-started labels Jan 31, 2017

cmsbuild added tests-started and removed tests-pending labels Jan 31, 2017

kpedro88 force-pushed the Phase1-HE15 branch from af175b1 to 74ad19e Compare January 31, 2017 16:37

cmsbuild added tests-approved and removed tests-started labels Jan 31, 2017

cmsbuild added comparison-available and removed comparison-pending labels Jan 31, 2017

davidlange6 merged commit fcab6f2 into cms-sw:CMSSW_9_0_X Jan 31, 2017

davidlange6 mentioned this pull request Feb 2, 2017

Pr90x L1T Stage2 for Phase2 #17248

Merged

davidlange6 mentioned this pull request Feb 20, 2017

HCAL dqm updates #17578

Closed

vkhristenko mentioned this pull request Feb 20, 2017

fixing the absence of const specifiers #17579

Merged

Fix or suppress warnings related to Phase0 HE in 2017 #17335

Fix or suppress warnings related to Phase0 HE in 2017 #17335

Conversation

kpedro88 commented Jan 31, 2017

cmsbuild commented Jan 31, 2017

kpedro88 commented Jan 31, 2017

cmsbuild commented Jan 31, 2017 • edited

davidlange6 commented Jan 31, 2017

kpedro88 commented Jan 31, 2017

davidlange6 commented Jan 31, 2017 via email

davidlange6 commented Jan 31, 2017 via email

kpedro88 commented Jan 31, 2017

cmsbuild commented Jan 31, 2017

kpedro88 commented Jan 31, 2017

cmsbuild commented Jan 31, 2017 • edited

dmitrijus commented Jan 31, 2017

rovere commented Jan 31, 2017

kpedro88 commented Jan 31, 2017

cmsbuild commented Jan 31, 2017

cmsbuild commented Jan 31, 2017

cmsbuild commented Jan 31, 2017

cmsbuild commented Jan 31, 2017

cmsbuild commented Jan 31, 2017

kpedro88 commented Jan 31, 2017

davidlange6 commented Jan 31, 2017

slava77 commented Feb 6, 2017

kpedro88 commented Feb 6, 2017

vkhristenko commented Feb 6, 2017

deguio commented Feb 7, 2017

davidlange6 commented Feb 17, 2017 via email

kpedro88 commented Feb 17, 2017

vkhristenko commented Feb 17, 2017

hatakeyamak commented Feb 17, 2017

dmitrijus commented Feb 20, 2017

davidlange6 commented Feb 20, 2017 via email

davidlange6 commented Feb 20, 2017

vkhristenko commented Feb 20, 2017 • edited

kpedro88 commented Feb 20, 2017

vkhristenko commented Feb 20, 2017

vkhristenko commented Feb 20, 2017

cmsbuild commented Jan 31, 2017 •

edited

cmsbuild commented Jan 31, 2017 •

edited

vkhristenko commented Feb 20, 2017 •

edited