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

First version of phase2 premixing #23065

Merged
merged 25 commits into from May 23, 2018
Merged

Conversation

makortel
Copy link
Contributor

@makortel makortel commented Apr 26, 2018

This PR adds premixing for phase2 D17 geometry detectors as described in these O&C week slides
https://indico.cern.ch/event/711343/#54-phase-2-premixing-status

As a summary the changes include

  • Customization of ECAL, HCAL, DT, CSC, RPC premixing for phase2
  • PreMixingPhase2TrackerWorker for phase2 tracker premixing
    • Both inner and outer tracker are treated like pixel is treated in phase0/1 premixing
    • In the standard digitizer changed the makeDigiSimLinks to be a single parameter to control the behaviour of all sub-digitizers instead of sub-digitizer specific parameters
  • PreMixingHGCalWorker for HGCal premixing
    • The state of the digi accumulator is stored
  • PreMixingCaloParticleWorker for CaloParticles and SimClusters
    • Similar to PreMixingTrackingParticleWorker
    • SimClusters need an additional "DetId->total sim enery" map to calculate the fractions
  • PreMixingGEMWorker and PreMixingME0Worker for GEM and ME0
    • Mixing the digis as with other muon detectors
    • May change in the future (to SimHits, see next point)
  • PreMixingCrossingFramePSimHitWorker to mix PCrossingFrame<PSimHit> (from pileup) and CrossingFrame<PSimHit> (from signal)
    • to mix SimHits for ME0 pseudo digis (used for L1)
  • Disable L1 from premixing stage1 (not needed)
  • Some cleanup in MixingWorker, CrossingFrame<T>, and PCrossingFrame<T>
  • Combined stage1+stage2 workflow, which is now also enabled in standard matrix
    • For this the existing stage1 and stage2 workflow numbers were decreased (.98->.97, 0.99->0.98)

There is still work to do (I'd be surprised if there were no bugs), but I believe integrating the first complete version has value (getting it reviewed, tested etc).

Tested in CMSSW_10_2_X_2018-04-23-2300 (rebased on top of CMSSW_10_2_X_2018-05-09-2300), no changes expected in existing workflows.

@kpedro88 @mdhildreth

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23065/4470

Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23065/4470/git-diff.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-23065/4470/git-diff.patch | patch -p1

You can run scram build code-checks to apply code checks directly

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @makortel (Matti Kortelainen) for master.

It involves the following packages:

Configuration/EventContent
Configuration/PyReleaseValidation
Configuration/StandardSequences
DQM/HcalTasks
DataFormats/HGCDigi
DataFormats/WrappedStdDictionaries
L1Trigger/Configuration
L1Trigger/TrackTrigger
RecoLocalCalo/HGCalRecProducers
RecoLocalMuon/RPCRecHit
RecoLocalTracker/SiPhase2Clusterizer
RecoLocalTracker/SiPixelClusterizer
RecoParticleFlow/PFClusterProducer
RecoParticleFlow/PFProducer
SimCalorimetry/Configuration
SimCalorimetry/HGCalSimProducers
SimDataFormats/CaloAnalysis
SimDataFormats/CrossingFrame
SimGeneral/CaloAnalysis
SimGeneral/Configuration
SimGeneral/MixingModule
SimGeneral/PreMixingModule
SimMuon/Configuration
SimMuon/GEMDigitizer
SimMuon/MCTruth
SimTracker/Configuration
SimTracker/SiPhase2Digitizer
SimTracker/TrackTriggerAssociation
SimTracker/TrackerHitAssociation
Validation/HGCalValidation
Validation/MuonGEMDigis

@kpedro88, @fabozzi, @nsmith-, @rekovic, @thomreis, @vanbesien, @perrotta, @civanch, @cmsbuild, @GurpreetSinghChahal, @davidlange6, @smuzaffar, @Dr15Jones, @mdhildreth, @jfernan2, @slava77, @fabiocos, @prebello, @vazzolini, @kmaeshima, @dmitrijus, @franzoni can you please review it and eventually sign? Thanks.
@echabert, @felicepantaleo, @abbiendi, @sviret, @Martin-Grunewald, @bsunanda, @pfs, @threus, @mmusich, @seemasharmafnal, @mmarionncern, @kreczko, @sethzenz, @battibass, @jhgoh, @lgray, @prolay, @HuguesBrun, @drkovalskyi, @dildick, @trocino, @vandreev11, @DryRun, @GiacomoSguazzoni, @rovere, @VinInn, @cseez, @bellan, @kpedro88, @deguio, @wmtford, @ebrondol, @dgulhan, @gbenelli, @wddgit, @edjtscott, @calderona, @dkotlins, @folguera, @cbernet, @gpetruc, @bachtis this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@makortel
Copy link
Contributor Author

@cmsbuild, please test workflow 250202.18,250202.181,250200.1,250402.0,20234.99

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 26, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/27661/console Started: 2018/04/26 18:09

@@ -0,0 +1,156 @@
#ifndef DataFormats_HGCDigi_PHGCSimAccumulator_h
#define DataFormats_HGCDigi_PHGCSimAccumulator_h
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file should likely be located somewhere in SimDataFormats instead. None of the existing sub-packages were crystal-clear match, so I ask for better suggestions.

@cmsbuild
Copy link
Contributor

cmsbuild commented May 22, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/28114/console Started: 2018/05/22 20:57

@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-23065/28114/summary.html

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /build/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-23065/20234.99_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D17PU_GenSimHLBeamSpotFull14+SingleNuE10_cf_2023D17PU_PremixHLBeamSpotFull14PU_Premix+DigiFullTriggerPUPRMXCombined_2023D17PU+RecoFullGlobalPUPRMX_2023D17PU+HARVESTFullGlobalPU_2023D17PU
  • /build/cmsbld/jenkins/workspace/compare-root-files-short-matrix/results/JR-comparison/PR-23065/250402.0_TTbar_13+FS_TTbar_13_PRMXUP15_PU25+HARVESTUP15FS+MINIAODMCUP15FS

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 31
  • DQMHistoTests: Total histograms compared: 2901712
  • DQMHistoTests: Total failures: 30
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2901492
  • DQMHistoTests: Total skipped: 190
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 30 files compared)
  • Checked 128 log files, 14 edm output root files, 31 DQM output files

@fabiocos
Copy link
Contributor

+operations

the changes to the StandardSequences look consistent with the purpose of the PR

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

@fabiocos
Copy link
Contributor

+1

from the review thread it appears there are some residual points to be clarified, but a RelVal test will be beneficial for this, and the code looks mature to be tested in the pre-release

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