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

Re-enable suppression of CSC digis without matching pretriggers and triggers #34435

Merged
merged 6 commits into from Jul 28, 2021
Merged

Re-enable suppression of CSC digis without matching pretriggers and triggers #34435

merged 6 commits into from Jul 28, 2021

Conversation

dildick
Copy link
Contributor

@dildick dildick commented Jul 10, 2021

PR description:

This PR should re-enable CSC digi packing with pretriggers and triggers. This was disabled in Run-2. In this minimal version packing of comparator digis is done in the entire chamber with a single pretrigger.

PR validation:

Ran runTheMatrix.py -l muonmc

5.1_TTbar+TTbarFS+HARVESTFS Step0-PASSED Step1-PASSED  - time date Fri Jul  9 19:58:47 2021-date Fri Jul  9 19:50:39 2021; exit: 0 0
20.0_SingleMuPt10+SingleMuPt10+DIGI+RECO+HARVEST Step0-PASSED Step1-PASSED Step2-PASSED Step3-PASSED  - time date Fri Jul  9 20:01:01 2021-date Fri Jul  9 19:50:39 2021; exit: 0 0 0 0
21.0_SingleMuPt100+SingleMuPt100+DIGI+RECO+HARVEST Step0-PASSED Step1-PASSED Step2-PASSED Step3-PASSED  - time date Fri Jul  9 20:00:42 2021-date Fri Jul  9 19:50:40 2021; exit: 0 0 0 0
22.0_SingleMuPt1000+SingleMuPt1000+DIGI+RECO+HARVEST Step0-PASSED Step1-PASSED Step2-PASSED Step3-PASSED  - time date Fri Jul  9 20:03:16 2021-date Fri Jul  9 19:50:40 2021; exit: 0 0 0 0
23.0_JpsiMM+JpsiMM+DIGI+RECO+HARVEST Step0-PASSED Step1-PASSED Step2-PASSED Step3-PASSED  - time date Fri Jul  9 20:08:09 2021-date Fri Jul  9 19:58:48 2021; exit: 0 0 0 0
25.0_TTbar+TTbar+DIGI+RECOAlCaCalo+HARVEST+ALCATT Step0-PASSED Step1-PASSED Step2-PASSED Step3-PASSED Step4-PASSED  - time date Fri Jul  9 20:16:28 2021-date Fri Jul  9 20:00:42 2021; exit: 0 0 0 0 0
30.0_ZMM+ZMM+DIGI+RECO+HARVEST Step0-PASSED Step1-PASSED Step2-PASSED Step3-PASSED  - time date Fri Jul  9 20:09:14 2021-date Fri Jul  9 20:01:01 2021; exit: 0 0 0 0
124.4_SinglePiPt10+SinglePiPt10FS Step0-PASSED  - time date Fri Jul  9 20:06:18 2021-date Fri Jul  9 20:03:17 2021; exit: 0
124.5_SinglePiPt100+SinglePiPt100FS Step0-PASSED  - time date Fri Jul  9 20:09:31 2021-date Fri Jul  9 20:06:19 2021; exit: 0
9 7 6 6 1 tests passed, 0 0 0 0 0 failed

In the DQM plots we should see a slight decrease of CSC digis after packing-unpacking due to requirement of triggers/pretriggers.

if this PR is a backport please specify the original PR and why you need to backport that PR:

N/A

Before submitting your pull requests, make sure you followed this checklist:

@barvic @ptcox

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34435/23855

  • This PR adds an extra 32KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @dildick (Sven Dildick) for master.

It involves the following packages:

  • DQM/CSCMonitorModule (dqm)
  • EventFilter/CSCRawToDigi (reconstruction)
  • SLHCUpgradeSimulations/Configuration (upgrade, simulation)

@perrotta, @civanch, @ahmad3213, @kmaeshima, @andrius-k, @kpedro88, @ErnestaP, @cmsbuild, @srimanob, @jfernan2, @mdhildreth, @slava77, @jpata, @rvenditti can you please review it and eventually sign? Thanks.
@barvic, @makortel, @JanFSchulte, @sviret, @VinInn, @Martin-Grunewald, @ptcox, @mmusich, @mtosi this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy, @perrotta you are the release manager for this.

cms-bot commands are listed here

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals RelVals-INPUT AddOn
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1d01a1/16693/summary.html
COMMIT: 7c37b4f
CMSSW: CMSSW_12_0_X_2021-07-11-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/34435/16693/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

  • 5.15.1_TTbar+TTbarFS+HARVESTFS/step1_TTbar+TTbarFS+HARVESTFS.log
  • 135.4135.4_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS/step1_ZEE_13+ZEEFS_13+HARVESTUP15FS+MINIAODMCUP15FS.log
  • 8.08.0_BeamHalo+BeamHalo+DIGICOS+RECOCOS+ALCABH+HARVESTCOS/step2_BeamHalo+BeamHalo+DIGICOS+RECOCOS+ALCABH+HARVESTCOS.log
Expand to see more relval errors ...

RelVals-INPUT

  • 6.06.0_SingleMuPt1+SingleMuPt1INPUT+DIGI+RECO+HARVEST/step2_SingleMuPt1+SingleMuPt1INPUT+DIGI+RECO+HARVEST.log
  • 8.08.0_BeamHalo+BeamHaloINPUT+DIGICOS+RECOCOS+ALCABH+HARVESTCOS/step2_BeamHalo+BeamHaloINPUT+DIGICOS+RECOCOS+ALCABH+HARVESTCOS.log
  • 10.010.0_ADDMonoJet_d3MD3+ADDMonoJet_d3MD3INPUT+DIGI+RECO+HARVEST/step2_ADDMonoJet_d3MD3+ADDMonoJet_d3MD3INPUT+DIGI+RECO+HARVEST.log
Expand to see more relval errors ...

AddOn Tests

  • fastsimcmsDriver.py TTbar_8TeV_TuneCUETP8M1_cfi --conditions auto:run1_mc --fast -n 100 --eventcontent AODSIM,DQM --relval 100000,1000 -s GEN,SIM,RECOBEFMIX,DIGI:pdigi_valid,L1,DIGI2RAW,L1Reco,RECO,EI,VALIDATION --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --datatier GEN-SIM-DIGI-RECO,DQMIO --beamspot Realistic8TeVCollision : FAILED - time: date Mon Jul 12 10:17:07 2021-date Mon Jul 12 10:17:01 2021 s - exit: 256
  • fastsim1cmsDriver.py TTbar_13TeV_TuneCUETP8M1_cfi --conditions auto:run2_mc_l1stage1 --fast -n 100 --eventcontent AODSIM,DQM --relval 100000,1000 -s GEN,SIM,RECOBEFMIX,DIGI:pdigi_valid,L1,DIGI2RAW,L1Reco,RECO,EI,VALIDATION --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --datatier GEN-SIM-DIGI-RECO,DQMIO --beamspot NominalCollision2015 --era Run2_25ns : FAILED - time: date Mon Jul 12 10:17:10 2021-date Mon Jul 12 10:17:05 2021 s - exit: 256
  • fastsim2cmsDriver.py TTbar_13TeV_TuneCUETP8M1_cfi --conditions auto:run2_mc --fast -n 100 --eventcontent AODSIM,DQM --relval 100000,1000 -s GEN,SIM,RECOBEFMIX,DIGI:pdigi_valid,L1,DIGI2RAW,L1Reco,RECO,EI,VALIDATION --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --datatier GEN-SIM-DIGI-RECO,DQMIO --beamspot NominalCollision2015 --era Run2_2016 : FAILED - time: date Mon Jul 12 10:17:13 2021-date Mon Jul 12 10:17:07 2021 s - exit: 256
Expand to see more addon errors ...

@dildick
Copy link
Contributor Author

dildick commented Jul 12, 2021

There is a typo. I fixed it locally, but failed to propagate it to the branch.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34435/23873

  • This PR adds an extra 12KB to repository

@cmsbuild
Copy link
Contributor

Pull request #34435 was updated. @perrotta, @civanch, @ahmad3213, @kmaeshima, @andrius-k, @kpedro88, @ErnestaP, @cmsbuild, @srimanob, @jfernan2, @mdhildreth, @slava77, @jpata, @rvenditti can you please check and sign again.

@dildick
Copy link
Contributor Author

dildick commented Jul 12, 2021

Note: this PR sets usePreTriggers to True after it had been set to False in #5034 (this commit https://github.com/cms-sw/cmssw/pull/5034/files#diff-084344f1809526bd6a23ba067534817ef74b449c89ab5a493458f8ee22284f65).

@srimanob
Copy link
Contributor

Please test

@civanch
Copy link
Contributor

civanch commented Jul 28, 2021

+1

@jfernan2
Copy link
Contributor

jfernan2 commented Jul 28, 2021

So just to confirm, from the reco side, the reco changes from CSC and downstream (tracks, PF candidates, jets) look expected and reasonable.

Sorry for being tedious but, what about changes in HGCAL, ECAL, EGamma, BTAG, BHadron, AlcaBeamMonitor, RecotauV,....?

@jpata
Copy link
Contributor

jpata commented Jul 28, 2021

Sorry for being tedious but, what about changes in HGCAL, ECAL, EGamma, BTAG, BHadron, AlcaBeamMonitor, RecotauV,....?

As far as I see, this is all downstream of track reconstruction, so small differences can be expected.

  • ECAL: the DQM ECAL folder reports only differences in reduced rechits, which come from ReducedRecHitCollectionProducer (link), which uses tracks and muons
  • HGCAL: the folder reports differences in HGCAL PFCandidates
  • EGamma, BTAG, BHadron, AlcaBeamMonitor, RecotauV are also downstream of tracks

@jpata
Copy link
Contributor

jpata commented Jul 28, 2021

+reconstruction

  • PR description is in line with the changes
  • physics changes expected in CSCs (less CSC reconstructed), and thus downstream in tracks and everything that relies on tracks (many small differences across the board)

@jfernan2
Copy link
Contributor

Thanks @jpata However I am still not sure why Pixel or Sitrip should reflect changes in ClusterCharges or hits due to this PR
I am totally puzzled, sorry if I missed the explanation somehow along the thread, I guess the full detector is interconnected specially when it comes about tracks matching hits from muons but I don't understand how TIB can use hits from CSC e.g.:
https://tinyurl.com/ygheevkz

@mmusich
Copy link
Contributor

mmusich commented Jul 28, 2021

I guess the full detector is interconnected specially when it comes about tracks matching hits from muons but I don't understand how TIB can use hits from CSC.

These are on-track hits. If the tracks are different for whatever reason (I presume here because the muon-seeded iterations are different due to changes in the CSC) also the hit composition of the track will change.

@ptcox
Copy link
Contributor

ptcox commented Jul 28, 2021

I have two comments on this thread

  1. It is not 'zero suppression'. There are no 'zeroes' getting suppressed. It is suppression of strip hits for which there is basically no associated L1 trigger primitive. As in the real detector: readout is suppressed if there is no pattern of a preCLCT, part of a cathode( = strips) trigger primitive. This is basically a hit on at least 3 layers of a 6-layer CSC, in a straight line vaguely pointing to the IP). [Sven can give precise details.]

  2. This readout suppression cannot directly affect any other part of CMS. However, if simulated CSC hits are not readout in one case, and are in another, one could in one case not reconstruct a CSC segment which otherwise appears and is reconstructed. That can then affect full Muon track reconstruction. It is most likely to affect 'Tracker Muons' in which one has a Tracker Track matched to a CSC segment in (possibly) only one Muon station. If that segment is suppressed, because the hits were readout-suppressed, then that Tracker Muon will no longer appear.
    How that impinges on other subdetectors in CMS depends on what use they make of reco'ed muons. For example, it could be some subdetector selects muons to study properties of that detector. If so, they'll obviously expect some loss of those muons when CSC readout suppression is enabled.

@jfernan2
Copy link
Contributor

Well, answering myself I see there may be (muon)tracks registered in TIB or Pixel passing trhough CSC too... Even TOB which is less likely but it could be true in the first two layers for a muon at eta~1.2 like in
https://tinyurl.com/ye2kcgoa

@jfernan2
Copy link
Contributor

+1

@jpata
Copy link
Contributor

jpata commented Jul 28, 2021

It is not 'zero suppression'. There are no 'zeroes' getting suppressed.

Perhaps the title can be changed to reduce the confusion, @dildick?

@dildick dildick changed the title Re-enable CSC digi zero-suppression with pretriggers and triggers Re-enable suppression of CSC digis when they are not spatially matched with pretriggers and triggers Jul 28, 2021
@dildick dildick changed the title Re-enable suppression of CSC digis when they are not spatially matched with pretriggers and triggers Re-enable suppression of CSC digis without matching pretriggers and triggers Jul 28, 2021
@qliphy
Copy link
Contributor

qliphy commented Jul 28, 2021

@cms-sw/upgrade-l2 Do you have any comment?

@srimanob
Copy link
Contributor

+Upgrade

@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. @silviodonato, @dpiparo, @qliphy, @perrotta (and backports should be raised in the release meeting by the corresponding L2)

@qliphy
Copy link
Contributor

qliphy commented Jul 28, 2021

+1

@cmsbuild cmsbuild merged commit 2398d9b into cms-sw:master Jul 28, 2021
@dildick dildick deleted the from-CMSSW_12_0_X_2021-07-09-0800-test-packing-unpacking branch July 28, 2021 14:16
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