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

Save L1 stage2 info in offline data tiers (81X) #14051

Merged
merged 3 commits into from Apr 22, 2016

Conversation

gpetruc
Copy link
Contributor

@gpetruc gpetruc commented Apr 13, 2016

Add L1 info from Stage2 unpackers to all the data tiers: L1 bits from GT, L1 objects from GMT and Calo. This works if era Run2_2016, or any other era that satisfies eras.stage2L1Trigger.isChosen().

Checked on CMSSW_8_1_X_2016-04-13-1100 for matrix workflow 1324.0, verifying by hand in that in those events the L1 muons and jets in the MiniAOD file are in agreement with the corresponding offline objects. Will do a sanity check on data on run 269610 too.

I did not remove the old L1 objects from the event content, as I've seen that in the raw2digi sequence the reco is configured to unpack both the old and the new L1.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @gpetruc (Giovanni Petrucciani) for CMSSW_8_1_X.

It involves the following packages:

L1Trigger/Configuration
PhysicsTools/PatAlgos

@cvuosalo, @cmsbuild, @rekovic, @slava77, @mulhearn, @davidlange6 can you please review it and eventually sign? Thanks.
@TaiSakuma, @rappoccio, @imarches, @ahinzmann, @acaudron, @mmarionncern, @Martin-Grunewald, @jdolen, @nhanvtran, @schoef, @ferencek, @mariadalfonso, @pvmulder 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

if eras.stage2L1Trigger.isChosen():
# stage 2 L1 trigger
l1Stage2Digis = [
'keep *_gtStage2Digis__*',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there should be a star between the two underscores....

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not put the star on purpose, as my understanding from the email thread was that for the GT we wanted only the bits (GlobalAlgBlk and GlobalExtBlk) which have an empty instance label, and not also all the other collections (muons, jets, taus, etc).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure it is the correct syntax in this case?

@slava77
Copy link
Contributor

slava77 commented Apr 13, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

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

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Pull request #14051 was updated. @cvuosalo, @cmsbuild, @rekovic, @slava77, @mulhearn, @davidlange6 can you please check and sign again.

@gpetruc
Copy link
Contributor Author

gpetruc commented Apr 14, 2016

Updated using the era.toModify as suggested by @slava77 (and tested also on data using cmsDriver, run 269610).

Shall I also make the PR for 80X? I've tested the re-based version of this in 80X on MC with cmsDriver taking the relval commands but udating the era to Run2_2016, and on data with RunPromptReco, and it seems to work fine in both cases.

@slava77
Copy link
Contributor

slava77 commented Apr 14, 2016

@cmsbuild please test

@gpetruc please make a PR for 80X
(I tested this code in 80X yesterday; needed a file with the right output for a somewhat related reason)

@cmsbuild
Copy link
Contributor

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

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@slava77
Copy link
Contributor

slava77 commented Apr 14, 2016

+1

for #14051 9f216c9

  • code changes are as described
  • jenkins tests pass
    • comparisons with baseline show no difference, as expected: new collections addition didn't trigger downstream changes
    • jenkins log files for reco step in 2016 MC workflows now show warnings from L1TRawToDigi:caloStage2Digis that the data is not there
Cannot unpack: empty L1T raw data (size = 0) for ID 1360. Returning empty collections!
- jenkins output files show new collections in reco/AOD and miniAOD files 
- local test of data from 269223/MinimumBias with 5K events show that the new collections add ~0.7kB to the MINIAOD event content (on 10 events in jenkins tests this number is 40kB due to rather poor compression). This will likely be a bit bigger (?x2) in more realistic collision events.

@rekovic
Copy link
Contributor

rekovic commented Apr 21, 2016

+1

Cannot unpack: empty L1T raw data (size = 0) for ID 1360. Returning empty collections!

is understood and is not relevant.

The Calo unpacker is currently using both FED ID 1360 (empty) and 1366 (correct),
and it will be configured only to use ID 1366.

@davidlange6 davidlange6 merged commit 7e4bacb into cms-sw:CMSSW_8_1_X Apr 22, 2016
@gpetruc gpetruc deleted the saveL1Stage2Info-81X branch June 1, 2016 07:41
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

6 participants