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
Phase2 workflow fixes #14165
Phase2 workflow fixes #14165
Conversation
A new Pull Request was created by @kpedro88 (Kevin Pedro) for CMSSW_8_1_X. It involves the following packages: Configuration/Geometry @civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @fabozzi, @cmsbuild, @srimanob, @franzoni, @slava77, @hengne, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are list here #13028 |
@cmsbuild please test |
The tests are being triggered in jenkins. |
@@ -63,3 +63,6 @@ | |||
chi2ThreshEE_ = cms.double(50.0), | |||
) | |||
) | |||
|
|||
from Configuration.StandardSequences.Eras import eras | |||
eras.phase2_common.toModify( ecalMultiFitUncalibRecHit.algoPSet, useLumiInfoRunHeader = cms.bool(False) ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The underlying problem is that bunchSpacingProducer is separate from localreco:
https://github.com/cms-sw/cmssw/blob/CMSSW_8_1_X/Configuration/StandardSequences/python/Reconstruction_cff.py#L101
Without it, there is an exception when running ecalMultiFitUncalibRecHit. The above customization seemed like the easiest way to prevent the exception. I'm open to other solutions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add it to the sequence you are running (at the start).
It's already in the localreco in HI and cosmics.
There is no harm from listing the same module several times in different sequences
-1 Tested at: e26834b You can see the results of the tests here: I found follow errors while testing this PR Failed tests: RelVals AddOn
When I ran the RelVals I found an error in the following worklfows: runTheMatrix-results/140.53_RunHI2011+RunHI2011+RECOHID11+HARVESTDHI/step2_RunHI2011+RunHI2011+RECOHID11+HARVESTDHI.log1000.0 step2 runTheMatrix-results/1000.0_RunMinBias2011A+RunMinBias2011A+TIER0+SKIMD+HARVESTDfst2+ALCASPLIT/step2_RunMinBias2011A+RunMinBias2011A+TIER0+SKIMD+HARVESTDfst2+ALCASPLIT.log1001.0 step2 runTheMatrix-results/1001.0_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVD1+ALCAHARVD2+ALCAHARVD3+ALCAHARVD4/step2_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVD1+ALCAHARVD2+ALCAHARVD3+ALCAHARVD4.log1003.0 step2 runTheMatrix-results/1003.0_RunMinBias2012A+RunMinBias2012A+RECODDQM+HARVESTDDQM/step2_RunMinBias2012A+RunMinBias2012A+RECODDQM+HARVESTDDQM.log
I found errors in the following addon tests: cmsDriver.py RelVal -s HLT:PRef,RAW2DIGI,L1Reco,RECO --data --scenario=pp -n 10 --conditions auto:run2_data_PRef --relval 9000,50 --datatier "RAW-HLT-RECO" --eventcontent FEVTDEBUGHLT --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --era Run2_2016 --magField 38T_PostLS1 --processName=HLTRECO --filein file:RelVal_Raw_PRef_DATA.root --fileout file:RelVal_Raw_PRef_DATA_HLT_RECO.root : FAILED - time: date Wed Apr 20 14:15:21 2016-date Wed Apr 20 14:08:39 2016 s - exit: 16640 |
looks like the errors are not related to this PR (other PRs are suffering from the same errors ..) |
@boudoul - yes, I've seen them in other PRs also. It appears to be a problem with jet corrections:
|
Pull request #14165 was updated. @civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @fabozzi, @cmsbuild, @srimanob, @franzoni, @slava77, @hengne, @davidlange6 can you please check and sign again. |
@slava77 latest commit addresses your recommendation about bunchSpacingProducer |
@@ -194,4 +196,5 @@ | |||
# | |||
reconstruction_standard_candle = cms.Sequence(localreco*globalreco*vertexreco*recoJetAssociations*btagging*electronSequence*photonSequence) | |||
|
|||
|
|||
from Configuration.StandardSequences.Eras import eras | |||
eras.phase2_common.toReplaceWith( localreco, _phase2_localreco ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe easier to add globally, no eras
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is that allowed? I don't want to screw up other workflows
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be OK, we'll see in the tests anyways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
submitted as a separate commit so it can be reverted back to the Era-dependent version if necessary
Update on known warning 2: it's unrelated to Phase2, caused by a change in the external. I submitted an issue against the external: cms-externals/CSCTrackFinderEmulation#3 |
Pull request #14165 was updated. @civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @fabozzi, @cmsbuild, @srimanob, @franzoni, @slava77, @hengne, @davidlange6 can you please check and sign again. |
@kpedro88 , cms-externals/CSCTrackFinderEmulation tool file is updated to have the correct path. Next IB (23h00) should have the fix. |
@smuzaffar great, thanks |
@cmsbuild please test |
The tests are being triggered in jenkins. |
The comparison looks good to me. @davidlange6 @slava77 @civanch @ianna @hengne - can you sign ASAP so we can make 810pre3? |
all - will merge this tonight as to build pre3 unless there are issues raised in the meantime |
@davidlange6 did you mean to merge this? |
Ah I see, it was included in #14181 |
This is the promised "workflow fix" PR for Phase2. This is just meant to get some things nominally working before 8_1_0_pre3. Further reorganizations will come later.
Major changes:
Various bug fixes/customizations/patches:
useLumiInfoRunHeader
inecalMultiFitUncalibRecHit
to fix ECALbunchSpacingProducer
exceptionecalRecHit
to avoid use of EcalRecHitWorkerRecover, which has non-optional use of EcalEndcapGeometryRecord that causes an exception in Phase2 geometries (a better fix for this should come later)Known warnings:
/cvmfs/cms-ib.cern.ch/2016-17/slc6_amd64_gcc530/cms/CSCTrackFinderEmulation/1.2/data/L1Trigger/CSCTrackFinder/data/core_2014_05_15/comp_dphi_8.dat
with a slash between data and L1Trigger:I tested this using the normal and tilted FourMuPt1_200 upgrade workflows:
10 events run to completion for steps 1 and 2 with no crashes or exceptions.
attn: @boudoul, @lgray, @bsunanda, @ianna, @calabria
@davidlange6 - should we add upgrade workflows into the PR/IB standard tests in this PR, or wait?