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.

Sign up for GitHub

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

Jump to bottom

updates for 2016 (Stage-2) L1T calorimeter trigger #13067

Merged
merged 4 commits into from Feb 4, 2016
Merged

updates for 2016 (Stage-2) L1T calorimeter trigger #13067

merged 4 commits into from Feb 4, 2016

Conversation

mulhearn
Copy link
Contributor

Updates to the Stage2 calo emulation necessary for the 2016 run.

Please note there is no change to the persistent data in CaloParams class.

@mulhearn
updates for 2016 (Stage-2) L1T calorimeter trigger
370787f 
Conflicts:
	CondFormats/L1TObjects/interface/CaloParams.h
	L1Trigger/L1TCalorimeter/interface/CaloParamsHelper.h
	L1Trigger/L1TCalorimeter/interface/Stage2Layer2TauAlgorithmFirmware.h
	L1Trigger/L1TCalorimeter/plugins/L1TCaloParamsESProducer.cc
	L1Trigger/L1TCalorimeter/plugins/L1TStage2Layer2Producer.cc
	L1Trigger/L1TCalorimeter/src/firmware/Stage1Layer2EtSumAlgorithmImpHI.cc
	L1Trigger/L1TCalorimeter/src/firmware/Stage2Layer2DemuxEGAlgoFirmwareImp1.cc
	L1Trigger/L1TCalorimeter/src/firmware/Stage2Layer2DemuxSumsAlgoFirmwareImp1.cc
	L1Trigger/L1TCalorimeter/src/firmware/Stage2Layer2DemuxTauAlgoFirmwareImp1.cc
	L1Trigger/L1TCalorimeter/src/firmware/Stage2Layer2EGammaAlgorithmFirmwareImp1.cc
	L1Trigger/L1TCalorimeter/src/firmware/Stage2Layer2EtSumAlgorithmFirmwareImp1.cc
	L1Trigger/L1TCalorimeter/src/firmware/Stage2Layer2JetAlgorithmFirmwareImp1.cc
	L1Trigger/L1TCalorimeter/src/firmware/Stage2Layer2JetSumAlgorithmFirmwareImp1.cc
	L1Trigger/L1TCalorimeter/src/firmware/Stage2Layer2TauAlgorithmFirmwareImp1.cc
	L1Trigger/L1TCalorimeter/src/firmware/Stage2TowerCompressAlgorithmFirmwareImp1.cc
	L1Trigger/L1TCalorimeter/src/firmware/Stage2TowerDecompressAlgorithmFirmwareImp1.cc
@cmsbuild cmsbuild added this to the Next CMSSW_8_0_X milestone Jan 26, 2016
@cmsbuild
Copy link
Contributor

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

It involves the following packages:

CondFormats/L1TObjects
L1Trigger/L1TCalorimeter

@diguida, @cerminar, @cmsbuild, @franzoni, @ggovi, @mmusich, @mulhearn, @davidlange6 can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @apfeiffer1, @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

This was referenced Jan 26, 2016
Merged
Closed
@mulhearn
Copy link
Contributor Author

please test

@mulhearn
Copy link
Contributor Author

+1

@cmsbuild
Copy link
Contributor

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

@cmsbuild
Copy link
Contributor

-1
Tested at: 370787f
When I ran the RelVals I found an error in the following worklfows:
135.4 step1

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

1306.0 step2

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

1330.0 step2

runTheMatrix-results/1330.0_ZMM_13+ZMM_13+DIGIUP15+RECOUP15+HARVESTUP15/step2_ZMM_13+ZMM_13+DIGIUP15+RECOUP15+HARVESTUP15.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

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

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

@mulhearn
Copy link
Contributor Author

Ah, right, stupid. We need to keep the previous version of configs around until PR for standard sequence is in place. Fixing...

@mulhearn
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

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

@mulhearn
Copy link
Contributor Author

mulhearn commented Feb 4, 2016

@ggovi note that this version (which passes tests, including reading Stage-1 from DB) has pnode size set to zero in default ctor... the enum is no longer in CondFormats.

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 4, 2016

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

@mulhearn
Copy link
Contributor Author

mulhearn commented Feb 4, 2016

+1

@ggovi
Copy link
Contributor

ggovi commented Feb 4, 2016

@mulhearn
you said: "That simply cannot be. Are you telling me that if the default ctor for a dataformat class initializes an std::vector to be five long, and my user code then adds five more elements before writing, reading from the DB will then crash?"
That's not the case. The concern is: how does the de-serialization fill the vector? Probably re-sizing according to the data read out. It could be ok, but I would avoid pre-sizing.

@mulhearn
Copy link
Contributor Author

mulhearn commented Feb 4, 2016

@ggovi this version (which passes tests) sets the initial size to zero, exactly as you recommend. Note that this means that in the tests, it successfully deserializes the Stage-1 output into a pnode vector which was set initially to zero. I think we are good, here, no?

@mulhearn
Copy link
Contributor Author

mulhearn commented Feb 4, 2016

@davidlange6 are you convinced?

@ggovi
Copy link
Contributor

ggovi commented Feb 4, 2016

These changes are borderline wrt our policy. Vectors are common and legitimate as persistent data members, but it's up the class 'interface' ensure that the only the available elements are accessed. The consumers should be protected in this sense.
Anyhow, reading back the payloads from the DB works - I mean, what is loaded in memory corresponds to the data stored in the DB.

@ggovi
Copy link
Contributor

ggovi commented Feb 4, 2016

+1

@mmusich
Copy link
Contributor

mmusich commented Feb 4, 2016

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 4, 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

@mulhearn
Copy link
Contributor Author

mulhearn commented Feb 4, 2016

@davidlange6 do you think that this can make the next IB? (It would make the remaining PRs much easier to integrate.)

@davidlange6
Copy link
Contributor

+1
ok, lets see..

cmsbuild added a commit that referenced this pull request Feb 4, 2016
@cmsbuild
Merge pull request #13067 from cms-l1t-offline/pr-layer2stage2-80x
08a989f 
updates for 2016 (Stage-2) L1T calorimeter trigger
@cmsbuild cmsbuild merged commit 08a989f into cms-sw:CMSSW_8_0_X Feb 4, 2016
cmsbuild added a commit that referenced this pull request Feb 4, 2016
@cmsbuild
Merge pull request #13067 from cms-l1t-offline/pr-layer2stage2-80x
47e8385
@mulhearn mulhearn deleted the pr-layer2stage2-80x branch February 11, 2016 23:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
alca-approved comparison-available db-approved fully-signed l1-approved orp-approved tests-approved
Projects
None yet
Milestone
CMSSW_8_0_X
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@mulhearn @cmsbuild @ggovi @mmusich @davidlange6