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

EmDQM HLT validation FastSim fix #5859

Merged
merged 1 commit into from Oct 23, 2014

Conversation

thomreis
Copy link
Contributor

Hard coded genParticles process name resulted in empty histograms in FastSim after the consumes migration in 7_0_0_pre10.

This affects all Egamma HLT FastSim Relval starting from 7_0_0_pre10.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @thomreis (Thomas Reis) for CMSSW_7_2_X.

EmDQM HLT validation FastSim fix

It involves the following packages:

HLTriggerOffline/Egamma

@nclopezo, @danduggan, @rovere, @cmsbuild, @deguio, @ojeda can you please review it and eventually sign? Thanks.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@nclopezo you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@deguio
Copy link
Contributor

deguio commented Oct 17, 2014

@thomreis please submit for 73 also.
thanks,
F.

@thomreis
Copy link
Contributor Author

@deguio you mean a new pull request for 73?
Looks like I have to resolve some conflicts then.

@deguio
Copy link
Contributor

deguio commented Oct 17, 2014

just want to make sure that this change doesn't get lost in 72 and it is ported to the dev branch as well.
another PR with the same change to line:
https://github.com/cms-sw/cmssw/blob/CMSSW_7_3_X/HLTriggerOffline/Egamma/src/EmDQM.cc#L48

is needed. you shouldn't find conflicts.
thanks,
F.

@thomreis
Copy link
Contributor Author

OK I have made a new PR #5868 for 73 with the same changes.
Is this change also going to be in future patch releases of 72? Otherwise we cannot do the FastSim validation for Egamma HLT paths.

@deguio
Copy link
Contributor

deguio commented Oct 17, 2014

+1
yes, as soon as it is merged, this change will be part of the next 72 releases

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_2_X IBs unless changes or unless it breaks tests. @nclopezo can you please take care of it?

@cmsbuild
Copy link
Contributor

-1
Tested at: 37aea8a
When I ran the RelVals I found an error in the following worklfows:
4.53 step2

runTheMatrix-results/4.53_RunPhoton2012B+RunPhoton2012B+HLTD+RECODreHLT+HARVESTDreHLT/step2_RunPhoton2012B+RunPhoton2012B+HLTD+RECODreHLT+HARVESTDreHLT.log
----- Begin Fatal Exception 17-Oct-2014 11:34:53 CEST-----------------------
An exception of category 'InvalidInput' occurred while
   [0] Processing run: 194533 lumi: 329 event: 461582793
   [1] Running path 'HLT_PFJet40_v1'
   [2] Calling event method for module PFJetCorrectionProducer/'hltAK4PFJetsCorrected'
   [3] Using EventSetup component JetCorrectionESChain/'hltESPAK4PFCorrection' to make data JetCorrector/'hltESPAK4PFCorrection' in record JetCorrectionsRecord
   [4] Using EventSetup component LXXXCorrectionESProducer/'hltESPAK4PFAbsoluteCorrectionESProducer' to make data JetCorrector/'hltESPAK4PFAbsoluteCorrectionESProducer' in record JetCorrectionsRecord
Exception Message:
 cannot find key 2 in the JEC payload, this usually means you have to change the global tag
----- End Fatal Exception -------------------------------------------------

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

@davidlange6
Copy link
Contributor

@cmsbuild - could you rerun the tests

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_2_X IBs unless changes (tests are also fine). @nclopezo can you please take care of it?

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_2_X IBs unless changes (tests are also fine). @nclopezo can you please take care of it?

@davidlange6
Copy link
Contributor

perhaps going back to the pre getbytoken design of having this InputTag as part of the PSet would be a more robust long term solution?

davidlange6 added a commit that referenced this pull request Oct 23, 2014
@davidlange6 davidlange6 merged commit 0b13d1f into cms-sw:CMSSW_7_2_X Oct 23, 2014
@thomreis
Copy link
Contributor Author

I consiered an InputTag but then decided to go back to the similar version as before the getbytoken, which was also hardcoded, so that the configuration stays the same.
But I agree that using an inputTag is nicer.

@thomreis thomreis deleted the EmDQM_FastSim_fix branch October 23, 2014 09:38
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

4 participants