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

bsunanda:Run2-hcx65 Attempt to make LUT coder to be flexible for geometry changes #13205

Merged
merged 2 commits into from Feb 9, 2016

Conversation

bsunanda
Copy link
Contributor

@bsunanda bsunanda commented Feb 5, 2016

Gets all relevant information from HcalTopology

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2016

A new Pull Request was created by @bsunanda for CMSSW_8_0_X.

It involves the following packages:

CalibCalorimetry/HcalTPGAlgos
CalibCalorimetry/HcalTPGEventSetup

@diguida, @cerminar, @cmsbuild, @franzoni, @mmusich, @mulhearn, @davidlange6 can you please review it and eventually sign? Thanks.
@tocheng this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@bsunanda
Copy link
Contributor Author

bsunanda commented Feb 5, 2016

@cmsbuild Please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2016

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2016

-1
Tested at: 0b6e364
When I ran the RelVals I found an error in the following worklfows:
8.0 step2

runTheMatrix-results/8.0_BeamHalo+BeamHalo+DIGICOS+RECOCOS+ALCABH+HARVESTCOS/step2_BeamHalo+BeamHalo+DIGICOS+RECOCOS+ALCABH+HARVESTCOS.log

5.1 step1

runTheMatrix-results/5.1_TTbar+TTbarFS+HARVESTFS/step1_TTbar+TTbarFS+HARVESTFS.log

9.0 step2

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

135.4 step1

runTheMatrix-results/135.4_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS/step1_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS.log

25.0 step2

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

1306.0 step2

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

101.0 step1

runTheMatrix-results/101.0_SingleElectronE120EHCAL+SingleElectronE120EHCAL/step1_SingleElectronE120EHCAL+SingleElectronE120EHCAL.log

1330.0 step2

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

50202.0 step2

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

25202.0 step2

runTheMatrix-results/25202.0_TTbar_13+TTbar_13+DIGIUP15_PU25+RECOUP15_PU25+HARVESTUP15_PU25/step2_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-13205/11021/summary.html

@kpedro88
Copy link
Contributor

kpedro88 commented Feb 5, 2016

@bsunanda - I checked the segfault in gdb. With the new code:

nluts = 6912
getLUTId(1,-23,1,1) = 6912

So either the nluts calculation or the getLUTId calculation is not correct... can you check it?

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2016

Pull request #13205 was updated. @diguida, @cerminar, @cmsbuild, @franzoni, @mmusich, @mulhearn, @davidlange6 can you please check and sign again.

@bsunanda
Copy link
Contributor Author

bsunanda commented Feb 5, 2016

@cmsbuild Please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2016

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

@bsunanda
Copy link
Contributor Author

bsunanda commented Feb 5, 2016

@kpedro88 The crash is cured. Can you look into comparison plots.

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2016

@kpedro88
Copy link
Contributor

kpedro88 commented Feb 5, 2016

@bsunanda - the comparison looks fine to me. I am going to try to test this with my HF QIE10 private branch now to check if there will be further downstream changes needed to handle HF depths 3/4...

@franzoni
Copy link

franzoni commented Feb 8, 2016

Dear @bsunanda

as we reported at the ORM meeting last week, we AlCa deem that out signature should not be required for packages like this one which is a L1 emulation specific matter.

( incidentally: please note https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_8_0_X_2016-02-05-1100+13205/12368/validateJR/all_OldVSNew_RunHI2011wf140p53/all_OldVSNew_RunHI2011wf140p53.log )

As such we sign though the L1 signature is the only relevant one.

g.

@franzoni
Copy link

franzoni commented Feb 8, 2016

+1

@bsunanda
Copy link
Contributor Author

bsunanda commented Feb 8, 2016

@mulhearn Could you look into this PR. This PR basically removes hardcoded numbers

@mulhearn
Copy link
Contributor

mulhearn commented Feb 9, 2016

+1

@mulhearn
Copy link
Contributor

mulhearn commented Feb 9, 2016

Sorry for delayed... tied up yesterday.

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 9, 2016

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

@abdoulline
Copy link

In addition:
PR has been explicitly tested by Matthias Wolf with 2016 geometry for emulating fine-franularity 1x1 HF TrigPrims and it's all OK.

@davidlange6
Copy link
Contributor

+1

cmsbuild added a commit that referenced this pull request Feb 9, 2016
bsunanda:Run2-hcx65 Attempt to make LUT coder to be flexible for geometry changes
@cmsbuild cmsbuild merged commit 638172b into cms-sw:CMSSW_8_0_X Feb 9, 2016
@kpedro88 kpedro88 mentioned this pull request Feb 9, 2016
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