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

Prescale access for stage-2 environment (81X) #14312

Merged
merged 19 commits into from May 20, 2016

Conversation

Martin-Grunewald
Copy link
Contributor

Prescale access for stage-2 environment (81X)
It makes L1TGlobalUtil (stage-2) similar to L1GtUtils (legacy/stage-1) incl. helper class etc.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @Martin-Grunewald (Martin Grunewald) for CMSSW_8_1_X.

It involves the following packages:

HLTrigger/HLTcore
L1Trigger/L1TGlobal

@perrotta, @cmsbuild, @Martin-Grunewald, @rekovic, @fwyzard, @mulhearn, @davidlange6 can you please review it and eventually sign? Thanks.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@Martin-Grunewald
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 29, 2016

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

@cmsbuild
Copy link
Contributor

-1

Tested at: cbbcf77

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
b08c294
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-14312/12736/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-14312/12736/git-merge-result

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

I found follow errors while testing this PR

Failed tests: RelVals

  • RelVals:

When I ran the RelVals I found an error in the following worklfows:
5.1 step1

runTheMatrix-results/5.1_TTbar+TTbarFS+HARVESTFS/step1_TTbar+TTbarFS+HARVESTFS.log
8.0 step3
runTheMatrix-results/8.0_BeamHalo+BeamHalo+DIGICOS+RECOCOS+ALCABH+HARVESTCOS/step3_BeamHalo+BeamHalo+DIGICOS+RECOCOS+ALCABH+HARVESTCOS.log
1306.0 step3
runTheMatrix-results/1306.0_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15/step3_SingleMuPt1_UP15+SingleMuPt1_UP15+DIGIUP15+RECOUP15+HARVESTUP15.log
9.0 step3
runTheMatrix-results/9.0_Higgs200ChargedTaus+Higgs200ChargedTaus+DIGI+RECO+HARVEST/step3_Higgs200ChargedTaus+Higgs200ChargedTaus+DIGI+RECO+HARVEST.log
135.4 step3
runTheMatrix-results/135.4_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS/step3_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS.log
25.0 step3
runTheMatrix-results/25.0_TTbar+TTbar+DIGI+RECOAlCaCalo+HARVEST+ALCATT/step3_TTbar+TTbar+DIGI+RECOAlCaCalo+HARVEST+ALCATT.log
1330.0 step3
runTheMatrix-results/1330.0_ZMM_13+ZMM_13+DIGIUP15+RECOUP15+HARVESTUP15/step3_ZMM_13+ZMM_13+DIGIUP15+RECOUP15+HARVESTUP15.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

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
b08c294
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-14312/12736/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-14312/12736/git-merge-result

@cmsbuild
Copy link
Contributor

Pull request #14312 was updated. @perrotta, @cmsbuild, @Martin-Grunewald, @rekovic, @fwyzard, @mulhearn, @davidlange6 can you please check and sign again.

8 similar comments
@cmsbuild
Copy link
Contributor

Pull request #14312 was updated. @perrotta, @cmsbuild, @Martin-Grunewald, @rekovic, @fwyzard, @mulhearn, @davidlange6 can you please check and sign again.

@cmsbuild
Copy link
Contributor

Pull request #14312 was updated. @perrotta, @cmsbuild, @Martin-Grunewald, @rekovic, @fwyzard, @mulhearn, @davidlange6 can you please check and sign again.

@cmsbuild
Copy link
Contributor

Pull request #14312 was updated. @perrotta, @cmsbuild, @Martin-Grunewald, @rekovic, @fwyzard, @mulhearn, @davidlange6 can you please check and sign again.

@cmsbuild
Copy link
Contributor

Pull request #14312 was updated. @perrotta, @cmsbuild, @Martin-Grunewald, @rekovic, @fwyzard, @mulhearn, @davidlange6 can you please check and sign again.

@cmsbuild
Copy link
Contributor

Pull request #14312 was updated. @perrotta, @cmsbuild, @Martin-Grunewald, @rekovic, @fwyzard, @mulhearn, @davidlange6 can you please check and sign again.

@cmsbuild
Copy link
Contributor

Pull request #14312 was updated. @perrotta, @cmsbuild, @Martin-Grunewald, @rekovic, @fwyzard, @mulhearn, @davidlange6 can you please check and sign again.

@cmsbuild
Copy link
Contributor

Pull request #14312 was updated. @perrotta, @cmsbuild, @Martin-Grunewald, @rekovic, @fwyzard, @mulhearn, @davidlange6 can you please check and sign again.

@cmsbuild
Copy link
Contributor

Pull request #14312 was updated. @perrotta, @cmsbuild, @Martin-Grunewald, @rekovic, @fwyzard, @mulhearn, @davidlange6 can you please check and sign again.

@cmsbuild
Copy link
Contributor

Pull request #14312 was updated. @perrotta, @cmsbuild, @cvuosalo, @fwyzard, @Martin-Grunewald, @rekovic, @slava77, @mulhearn, @davidlange6 can you please check and sign again.

@Martin-Grunewald
Copy link
Contributor Author

please test

@Martin-Grunewald
Copy link
Contributor Author

@dmitrijus
Can you please now sign this PR as well?
Thanks!

@dmitrijus
Copy link
Contributor

+1

@Martin-Grunewald
Copy link
Contributor Author

Who signs for "analysis" these days?
Could you please do so asap?
Thanks!

@davidlange6
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 19, 2016

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

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@davidlange6
Copy link
Contributor

@Martin-Grunewald - is @gpetruc aware of what changes are apparently needed in PatTriggerProducer due to this PR? (given the new error messages introduced)

@gpetruc
Copy link
Contributor

gpetruc commented May 20, 2016

Hi,
I tested the 80X version of this (#14336) on 2016 data and it was ok, it was getting the proper prescales for HLT unlike what's currently in the release.
Giovanni

@Martin-Grunewald
Copy link
Contributor Author

@davidlange6 @gpetruc
Yes - I just forwarded you an e-mail exchange.

@Martin-Grunewald
Copy link
Contributor Author

For the record

 Subject: Re: PatTriggerProducer adapt to stage2

Hi,

Please have a look at:
80X: https://github.com/cms-sw/cmssw/pull/14336
81X: https://github.com/cms-sw/cmssw/pull/14312
and PhysicsTools/PatAlgos/python/triggerLayer1/triggerProducer_cfi.py
in there.

What needs to be done further:

PATTriggerProducer is calling L1GtUtils (stage-1/legacy).
For stage-2 this is replaced by L1TGlobalUtil instead,
which has a different interface to the user/client.

legact/stage-1 vs stage-2 should become configurable, and/or
one could use the new accessor "l1tType()" of the
HLTConfigProvider/HLTPrescaleProvider (in the above PRs) 
to find out what it is: 0=unknown, 1=legacy/stage-1, 2=stage-2.
This requires that the processName used is the correct one
for the process you want to analyse.

And there I think is a general problem: typically the processName
is "HLT" but it should be "reHLT" if one had been rerunning the
HLT and wants to run PATTriggerProducer on that HLT (use case of
data taken in 2015 with "HLT" stage-1 25ns, reprocessed through
the 2016 HLT using stage-2 L1T re-emulation). I believe this is
not really switched properly at all in relvals.

Relvals using HLT:@relval2016 and era Run2_2016 are configured
to use stage-2 L1T and corresponding HLT.

Best regards

Martin

On Wed, 11 May 2016, Slava Krutelyov wrote:

> Hi Giovanni,
> 
> On 5/11/16 12:11 PM, Giovanni Petrucciani wrote:
> > Hi,
> > 
> > I can check it for miniAOD production,
> 
> Thank you.
> 
> > 
> > My assumptions would be that we have to deal with 4 scenarios:
> > 1) MC with --era Run2_25ns : as in 2015
> > 2) MC with --era Run2_2016 : stage2
> > 3) 2016 Data prompt: stage 2
> > 4) 2016 Data re-reco: stage 2
> > 5) 2015 Data : as in 2015
> > 
> > Is it ok?
> > Can somebody provide a 80X MC file of scenario (2) with non-empty
> > trigger? (for testing (1) I suppose I could use 76X MC)
> 
> I think the latest 80X and/or 81X run2 workflows in MC are running
> stage2 emulator,
> but I'm not sure what's done at HLT.
> 
> > IMHO the important cases to check are (2) and (3, 4) since (1) and (5)
> > will only kick in if we'll use 80X on 2015 data.
> > Will data re-reco use eras as in MC?
> > 
> 
> These updates should at least not break the older workflows,
> they should keep running.
> It's a part of relval matrix.
> 
>               --slava
> 
> 
> > Giovanni
> > 
> > 
> > On Wed, May 11, 2016 at 11:42 AM, Slava Krutelyov <slava77@fnal.gov
> > <mailto:slava77@fnal.gov>> wrote:
> > 
> >     Hi Giovanni,
> > 
> >     Martin mentioned in the ORP yesterday that the PatTriggerProducer
> >     should be updated to appropriately pick up stage1 or stage2 or older
> >     run1 trigger objects.
> >     I understand that different options, stage2 more timely, has a
different
> >     class and interface to read the information from.
> >     Different values/options can be configured with eras to pick up the
> >     correct ones.
> >     Are you available to check what needs to be done and implement the
> >     changes?
> > 
> >     Please let me know.
> > 
> >     Thank you.
> > 
> >                     --slava
> > 
> > 
> > 
> >     --

@davidlange6
Copy link
Contributor

i mean specifically

%MSG-e HLTPrescaleProvider: PATTriggerProducer:patTrigger 19-May-2016 20:50:36 CEST Run: 1 Event: 1
Error in determining L1T prescale for HLT path: 'HLT_Mu3_PFJet40_v1' has multiple L1TSeed modules, 2, with L1T seeds: 'L1_SingleMu3' * 'L1_SingleJet35'. (Note: at most one L1TSeed module is allowed for a proper determination of the L1T prescale!)

@Martin-Grunewald
Copy link
Contributor Author

This message is limited to at most 2 per instance. It is up to the client (PATTriggerProducer)
to properly check first and call the code for these special cases of triggers having several L1T seed modules.

@gpetruc
Copy link
Contributor

gpetruc commented May 20, 2016

Yeah.

Funny enough, I'm even the one who coded that HLT path.

The prescale code at the moment can't understand that the L1_SingleJet35
instance used in the HLT_Mu3_PFJet40_v1 is taken from the object map and
not from the GT digis, and so it's bypassing masks and prescales.
In principle, it could do it by looking at the input tag used by the
HLTL1TSeed module (though that would mean putting the name of the digis
collection in the code, not a particularly nice solution).

In any case, all this affects the L1 prescales, which are currently not
available anyway because the corresponding data is not yet in conddb.
When the proper developments to access L1 prescales in CMSSW will be done,
we can fix this.

Ciao,

Giovanni

On Fri, May 20, 2016 at 9:37 AM, David Lange notifications@github.com
wrote:

i mean specifically

%MSG-e HLTPrescaleProvider: PATTriggerProducer:patTrigger 19-May-2016
20:50:36 CEST Run: 1 Event: 1
Error in determining L1T prescale for HLT path: 'HLT_Mu3_PFJet40_v1' has
multiple L1TSeed modules, 2, with L1T seeds: 'L1_SingleMu3' *
'L1_SingleJet35'. (Note: at most one L1TSeed module is allowed for a proper
determination of the L1T prescale!)


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#14312 (comment)

@davidlange6
Copy link
Contributor

ok, good enough for 81x

@davidlange6 davidlange6 merged commit 03fe3c2 into cms-sw:CMSSW_8_1_X May 20, 2016
@davidlange6
Copy link
Contributor

@mulhearn @gpetruc - I am reminded that for MC there is no "fix" needed to proceed as we want prescales=1. So, in order to get the MC release in shape, lets not couple the prescales from L1 with this fix. Thanks!

@Martin-Grunewald Martin-Grunewald deleted the Stage2Prescales81X branch June 1, 2016 04:48
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

9 participants