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

pr90x L1T GT packer of ExternalConditions #16794

Merged
merged 2 commits into from Dec 14, 2016

Conversation

rekovic
Copy link
Contributor

@rekovic rekovic commented Nov 29, 2016

90X PR: Needed for HI MC production.

Put back packing of ExternalConditions in packer GTSetup.
Configure default to use simGtExtFakeStage2Digis which is part of SimL1Emulator sequence by default.

Also fix L1REPACK:uGT configuration to use the same default ExtInputTag=simGtExtFakeStage2Digis.

…e default to use simGtExtFakeStage2Digis. Fix L1REPACK:uGT configuration to use the same default.
@rekovic
Copy link
Contributor Author

rekovic commented Nov 29, 2016

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 29, 2016

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

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @rekovic for CMSSW_9_0_X.

It involves the following packages:

Configuration/StandardSequences
EventFilter/L1TRawToDigi

@cmsbuild, @rekovic, @franzoni, @mulhearn, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @kreczko, @makortel, @felicepantaleo, @GiacomoSguazzoni, @rovere, @VinInn, @Martin-Grunewald, @dgulhan this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

@rekovic
Copy link
Contributor Author

rekovic commented Nov 29, 2016

+1

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

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

The workflows 1003.0, 1001.0, 1000.0, 140.53, 136.731, 4.22 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

@@ -46,7 +46,7 @@
packGtStage2.JetInputTag = cms.InputTag("unpackCaloStage2","Jet")
packGtStage2.EtSumInputTag = cms.InputTag("unpackCaloStage2","EtSum")
packGtStage2.GtInputTag = cms.InputTag("simGtStage2Digis") # as in default
packGtStage2.ExtInputTag = cms.InputTag("simGtStage2Digis") # as in default
packGtStage2.ExtInputTag = cms.InputTag("simGtExtFakeStage2Digis") # as in default
Copy link
Contributor

Choose a reason for hiding this comment

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

@rekovic - since I'm not sure about what monitoring we actually have in place - what is the effect of this change on pp MC (eg, the Moriond configuration)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@davidlange6
I did some investigation, and I started a github issue cms-l1t-offline#440. I expect @fwyzard / @Martin-Grunewald will comment.

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 9, 2016

Pull request #16794 was updated. @cmsbuild, @rekovic, @franzoni, @mulhearn, @davidlange6 can you please check and sign again.

@rekovic
Copy link
Contributor Author

rekovic commented Dec 9, 2016

+1

@rekovic
Copy link
Contributor Author

rekovic commented Dec 9, 2016

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 9, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/16898/console Started: 2016/12/09 14:57

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 9, 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 9, 2016

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 9, 2016

@rekovic
Copy link
Contributor Author

rekovic commented Dec 12, 2016

@davidlange6 This is all understood now. Can merge.
HLT for p-p does not use BPTX (Externals) and Moriond 2017 MC Production was validated as OK
without the ExternalConditions packed.

@davidlange6
Copy link
Contributor

i'm not sure I follow the thread - unless you are doing something with the random number seed (or running threaded), you expect 1 to 1 agreement unless something changes even if random numbers are used. can you clarify?

@rekovic
Copy link
Contributor Author

rekovic commented Dec 12, 2016

@davidlange6 double checking. will report again.

@fwyzard
Copy link
Contributor

fwyzard commented Dec 13, 2016

@davidlange6 I don't think the (lack of) reproducibility is tied to these changes; it is already there in CMSSW 8.0.24-patch1 .
Running the DIGI+L1 step with

cmsDriver.py test \
    -s DIGI:pdigi,L1 \
    --no_output \
    --python_filename=test.py \
    --processName HLTX \
    --conditions 80X_mcRun2_asymptotic_2016_TrancheIV_v6_Tr4GT_v6 \
    --era Run2_2016 \
    --filein /store/relval/CMSSW_8_0_21/RelValTTbar_13/GEN-SIM-DIGI-RAW-HLTDEBUG/PU25ns_80X_mcRun2_asymptotic_2016_TrancheIV_v6_Tr4GT_v6-v1/10000/049B4664-8A98-E611-8F0C-0025905B8562.root \
    -n 1000 \
    --customise=HLTrigger/Configuration/CustomConfigs.L1THLT

multiple times with 4 threads leads to different L1 results, but running only the L1 step with

cmsDriver.py test \
    -s L1 \
    --no_output \
    --python_filename=test.py \
    --processName HLTX \
    --conditions 80X_mcRun2_asymptotic_2016_TrancheIV_v6_Tr4GT_v6 \
    --era Run2_2016 \
    --filein /store/relval/CMSSW_8_0_21/RelValTTbar_13/GEN-SIM-DIGI-RAW-HLTDEBUG/PU25ns_80X_mcRun2_asymptotic_2016_TrancheIV_v6_Tr4GT_v6-v1/10000/049B4664-8A98-E611-8F0C-0025905B8562.root \
    -n 1000 \
    --customise=HLTrigger/Configuration/CustomConfigs.L1THLT

does not.

@davidlange6
Copy link
Contributor

davidlange6 commented Dec 13, 2016 via email

@fwyzard
Copy link
Contributor

fwyzard commented Dec 13, 2016

yes, if I run the DIGI+L1 job multiple times with a single thread I do get identical results.

@davidlange6
Copy link
Contributor

davidlange6 commented Dec 13, 2016 via email

@fwyzard
Copy link
Contributor

fwyzard commented Dec 13, 2016

Well, if the random number generator is per-module but global (as opposed to per-stream) it makes sense that running with multiple threads/streams can give different results across different jobs, since the order of execution of the events is not guaranteed, right ?
I guess one could have per-stream, per-module random number generators if reproducibility is a concern.

@fwyzard
Copy link
Contributor

fwyzard commented Dec 13, 2016

Scratch that - even if the generator were per-stream, the order of events across the streams is not guaranteed.

@Martin-Grunewald
Copy link
Contributor

Reproducibility with MT is important.
Yes, I can switch off MT, but maybe MT has/reveals other bugs (eg, modules not really MT safe)

@fwyzard
Copy link
Contributor

fwyzard commented Dec 13, 2016

Reproducibility with MT is important.
Yes, I can switch off MT, but maybe MT has/reveals other bugs (eg, modules not really MT safe)

Unfortunately I think in this case reproducibility cannot be guaranteed - it's a similar issue to what we have with the HLT prescales.

At least, I do not have any bright ideas about this...

@rekovic
Copy link
Contributor Author

rekovic commented Dec 13, 2016

@davidlange6
As mentioned to you on skype, irreproducibility has nothing to do with packing of the ExternalConditions per se (i.e. this PR). The irreproducibility was observed in some tests triggered by the code review comments of this PR. It is now concluded that the issue is the multithreading (MT).

Results of current unit tests of L1T (DIGI,L1T, DIGI,L1T,DIGI2RAW, DIGI,L1T,DIGI2RAW,RAW2DIGI are reproducible as they are not MT. At least MT is not turned on anywhere explicitely. Example unit-test:

test  -s DIGI:pdigi,L1,DIGI2RAW \
--conditions auto:run2_mc \
--datatier DIGI-RAW-HLTDEBUG \
 -n 1000 \
--era Run2_2016 \
--eventcontent RAW \
--filein /store/group/dpg_trigger/comm_trigger/TriggerStudiesGroup/STORM/GEN-SIM/CMSSW_8/06F2C3AC-8957-E611-9DDF-0025905B85D8.root \
--customise=L1Trigger/Configuration/customiseUtils.L1TGlobalDigisSummary \
--customise=L1Trigger/Configuration/customiseUtils.L1TAddInfoOutput

However, if L1THLT customisation is added, as in case of @fwyzard example above,

  --customise=HLTrigger/Configuration/CustomConfigs.L1THLT

the irrecproducibility is observed. Apparently, HLT configuration turns on the MT.

@davidlange6
Copy link
Contributor

Hi @rekovic - that seems to be correct, yes. (hopefully anyone using this customize in batch/grid is aware and changes their config accordingly)

@davidlange6 davidlange6 merged commit cddbf19 into cms-sw:CMSSW_9_0_X Dec 14, 2016
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

5 participants