Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HCAL DQM Update #12651

Merged
merged 12 commits into from Feb 16, 2016
Merged

Conversation

vkhristenko
Copy link
Contributor

This is the update for HCAL DQM in preparation for 2016.

Packages such as:
DQM/HcalCommon
DQM/HcalTasks
are new. 2 new configuration files, with exactly the same naming conventions as before:
DQM/Integration/python/clients/hcal_dqm_sourceclient-live_cfg.py
DQM/Integration/python/clients/hcalcalib_dqm_sourceclient-live_cfg.py

These new config files use only newly added packages.
Also, the DQMOffline/Configuration/python/DQMOffline_cff.py has been updated to stop using old packages for Offline (I mean modules coming from DQM/ only, none of modules from DQMOffline/ were touched).

runTheMatrix passed successfully.

To note, none of the packages sitting in DQM/, which are:
HcalMonitorTasks
HcalMonitorModule
HcalMonitorClient
will not be running any longer(at least that is the goal/plan!), with a single exception of 2 things:
DQM/HcalMonitorClient/python/HcalDataCertification_cfi.py
DQM/HcalMonitorClient/python/HcalDAQInfo_cfi.py
being used in Offline. That will be replaced at some point before the actual datataking

Submitting only for 80X as this is intended only for 2016.
VK

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 2, 2015

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

It involves the following packages:

DQM/HcalCommon
DQM/HcalTasks
DQM/Integration
DQMOffline/Configuration

@cmsbuild, @vanbesien, @deguio, @davidlange6 can you please review it and eventually sign? Thanks.
@threus, @batinkov, @rociovilar this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

Following commands in first line of a comment are recognized

  • +1|approve[d]|sign[ed]: L1/L2's to approve it
  • -1|reject[ed]: L1/L2's to reject it
  • assign <category>[,<category>[,...]]: L1/L2's to request signatures from other categories
  • unassign <category>[,<category>[,...]]: L1/L2's to remove signatures from other categories
  • hold: L1/all L2's/release manager to mark it as on hold
  • unhold: L1/user who put this PR on hold
  • merge: L1/release managers to merge this request
  • [@cmsbuild,] please test: L1/L2 and selected users to start jenkins tests
  • [@cmsbuild,] please test with cms-sw/cmsdist#<PR>: L1/L2 and selected users to start jenkins tests using externals from cmsdist

@vkhristenko
Copy link
Contributor Author

@abdoulline

@vanbesien
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 3, 2015

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 3, 2015

-1
Tested at: 9e346e8
When I ran the RelVals I found an error in the following worklfows:
9.0 step3

runTheMatrix-results/9.0_Higgs200ChargedTaus+Higgs200ChargedTaus+DIGI+RECO+HARVEST/step3_Higgs200ChargedTaus+Higgs200ChargedTaus+DIGI+RECO+HARVEST.log

25.0 step3

runTheMatrix-results/25.0_TTbar+TTbar+DIGI+RECOAlCaCalo+HARVEST+ALCATT/step3_TTbar+TTbar+DIGI+RECOAlCaCalo+HARVEST+ALCATT.log

1306.0 step3

runTheMatrix-results/1306.0_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15/step3_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15.log

1330.0 step3

runTheMatrix-results/1330.0_ZMM_13+ZMM_13+DIGIUP15+RECOUP15+HARVESTUP15/step3_ZMM_13+ZMM_13+DIGIUP15+RECOUP15+HARVESTUP15.log

50202.0 step3

runTheMatrix-results/50202.0_TTbar_13+TTbar_13+DIGIUP15_PU50+RECOUP15_PU50+HARVESTUP15_PU50/step3_TTbar_13+TTbar_13+DIGIUP15_PU50+RECOUP15_PU50+HARVESTUP15_PU50.log

25202.0 step3

runTheMatrix-results/25202.0_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25/step3_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25.log

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-12651/10106/summary.html

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 3, 2015

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

@vkhristenko
Copy link
Contributor Author

I've updated the imports in the rest of the DQMOffline... files
Only parts that import Online (DQM/HcalMonitor...) modules are replaced. None from DQMOffline/Hcal has been touched.

Tested against 25.0 that was causing issues

VK

@vanbesien
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 4, 2015

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

@cmsbuild
Copy link
Contributor

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

@deguio
Copy link
Contributor

deguio commented Feb 15, 2016

please test

@cmsbuild
Copy link
Contributor

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

@deguio
Copy link
Contributor

deguio commented Feb 15, 2016

+1
still waiting for the tests, but should be ok.

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_8_0_X IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @slava77, @davidlange6, @Degano, @smuzaffar

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@davidlange6
Copy link
Contributor

Finally catching up on this full PR. On the technical aspects of this code base I have a few comments below.

  • Most important (for this PR), we need to remove the try-catch blocks - at least in the offline workflow.
  • Second (needed for 8_0_1), there are a lot of detector properties defined in Constants.h. These are mostly (all?) defined elsewhere and should simply be taken from there. Eg, fed ids for hcal, sub detector enums, etc. Similarly, the validDetId function must already be implemented in hcal code - use it.
  • In the utilites functions, please pass the complex objects as const refs. I happened to notice that the 'ped' argument to maxTS has no influence on the outcome of the function.
    • Longer term, much more of this should be templated to remove duplicated code - I think that would make a large part of this completely independent of HCAL and thus could be moved up stream.

@davidlange6
Copy link
Contributor

@vkhristenko @deguio - sorry should have cc-ed you on the previous message in case you miss it.

@vkhristenko
Copy link
Contributor Author

@davidlange6 ,

  1. try - catch - ok - possible

  2. if there was any documentation - I would have done that - there is none. Previously all of that would be just hardcoded... Fed ids I know where to find and will update that, but the rest is not documented anywhere, at least there is no documentation that hcaldqm has....

  3. Sounds good

  4. Reagrding the templating - there is a reason why I didn't do that. I have had problems compiling the code when I use templating, would run out of memory - although that was done on some older machines..... but point taken!

David, what would be the timeline to fix (try-catch for instance) - a couple of days? say by thursday - or too late???

@davidlange6
Copy link
Contributor

On Feb 15, 2016, at 6:31 PM, Viktor Khristenko notifications@github.com wrote:

@davidlange6 ,

  1. try - catch - ok - possible

  2. if there was any documentation - I would have done that - there is none. Previously all of that would be just hardcoded... Fed ids I know where to find and will update that, but the rest is not documented anywhere, at least there is no documentation that hcaldqm has….

I guess the HCAL group can help with more expertise..but here are some examples.

https://github.com/cms-sw/cmssw/blob/CMSSW_8_0_X/DataFormats/FEDRawData/interface/FEDNumbering.h
https://github.com/cms-sw/cmssw/blob/CMSSW_8_0_X/DataFormats/HcalDetId/interface/HcalSubdetector.h

  1. Sounds good

  2. Reagrding the templating - there is a reason why I didn't do that. I have had problems compiling the code when I use templating, would run out of memory - although that was done on some older machines..... but point taken!

David, what would be the timeline to fix (try-catch for instance) - a couple of days? say by thursday - or too late???

thursday is good - I can merge this in if thats a doable timescale - then we can make a patch.


Reply to this email directly or view it on GitHub.

@vkhristenko
Copy link
Contributor Author

LOL - your 2 refs are the ones that I have used already lol... ok sounds good... I understood...
Yes, then by thursday morning I will provide an update with resolving try-catch issue in there!

VK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants