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

Simulation of the pixel bad components on the FED channel basis #25466

Merged

Conversation

tsusa
Copy link
Contributor

@tsusa tsusa commented Dec 11, 2018

The PR introduces a simulation of the pixel bad components on the FED channel bases (needed for the simulation of stuckTBMs and DCDC issue). To simulated FED bad channel the scenarios from Pixel PCL payloads are utilised. The approach was presented at the last simulation meeting https://indico.cern.ch/event/777956/contributions/3239525/attachments/1765581/2866399/tanjaSim_04112018.pdf.
The PR introduces the novel SiPixelFEDChannelContainer and SiPixelQualityProbabilities Condition formats.
The changes proposed here are:

  • added SiPixelFEDChannelContainer and SiPixelQualityProbabilities;
  • added corresponding records for the EventSetup, respectively SiPixelStatusScenariosRcd and SiPixelStatusScenarioProbabilityRcd;
  • added serialization tests for the new objects;
  • added payload writers and readers for the new CondFormats;
  • added utilities to import/fetch and dump payloads of the new type;

Caveat: SiPixelFEDChannelContainer is essentially a wrapper around PixelFEDChannel, which being a DataFormat necessitates its own (trivial) serialization/deserialization rules (implemented in CondFormats/External). As we don't foresee to ever have to change the content of the DataFormat schema evolution is implicitly avoided.

The changes are introduced in the SiPixelDigitizer, where in each event one scenario is randomly chosen and the bad components contained in the chosen scenario are killed. The information about killed components is propagated to the the tracking via SiPixelRawToDigi.
The new feature is switched off by default.

@mmusich @veszpv

@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 @tsusa (Tatjana Susa) for master.

It involves the following packages:

CondCore/CondDB
CondCore/SiPixelPlugins
CondCore/Utilities
CondFormats/DataRecord
CondFormats/External
CondFormats/SiPixelObjects
Configuration/AlCa
Configuration/Geometry
Configuration/PyReleaseValidation
Configuration/StandardSequences
DQM/HcalCommon
DQM/HcalTasks
EventFilter/SiPixelRawToDigi
FWCore/Concurrency
FWCore/Services
Geometry/CMSCommonData
Geometry/MTDCommonData
Geometry/MTDGeometryBuilder
Geometry/MTDNumberingBuilder
Geometry/VeryForwardGeometryBuilder
IOMC/EventVertexGenerators
L1Trigger/L1THGCal
L1Trigger/L1TMuonBarrel
RecoCTPPS/PixelLocal
RecoCTPPS/TotemRPLocal
RecoLocalCalo/HcalRecProducers
RecoTauTag/RecoTau
SimTracker/SiPixelDigitizer
Validation/Geometry

@andrius-k, @schneiml, @ianna, @kpedro88, @nsmith-, @rekovic, @thomreis, @pohsun, @perrotta, @civanch, @zhenhu, @cmsbuild, @davidlange6, @smuzaffar, @Dr15Jones, @cvuosalo, @mdhildreth, @jfernan2, @tocheng, @slava77, @ggovi, @fabiocos, @prebello, @kmaeshima, @pgunnell, @franzoni, @lpernie can you please review it and eventually sign? Thanks.
@felicepantaleo, @forthommel, @Martin-Grunewald, @jbsauvan, @thomreis, @rishabhCMS, @threus, @mmusich, @seemasharmafnal, @vargasa, @makortel, @lgray, @DryRun, @GiacomoSguazzoni, @rovere, @VinInn, @tocheng, @deguio, @ebrondol, @dgulhan, @rbartek, @jan-kaspar, @wddgit, @dkotlins, @mariadalfonso, @amarini 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

@slava77
Copy link
Contributor

slava77 commented Dec 11, 2018

something wrong happened with the commits.
perhaps this is due to the "global" issue that triggered many other PRs to be inconsistent/appearing to merge many unrelated PRs

@mmusich mmusich force-pushed the StuckTBM_from-CMSSW_10_4_X_2018-12-07-2300 branch from 4ee9a0d to 1508af4 Compare December 11, 2018 08:02
@smuzaffar
Copy link
Contributor

@ggovi , @perrotta @mmusich

  • Header Consistency: please ignore these, we get a lot of Internel compiler Errors, so this check can be ignore for now.
  • The duplicate dictionary failure is a real issue but not due to this PR. This problem exists in the IBs too. problem is [a] where we have a plugin definition in side a shared library. Any plugin which links against this shared library exposes this EcalPedestalHistory definition to plugin manager. @ggovi I would propose to move this definition in to CondTools/Ecal/plugins

[a]

CondTools/Ecal/src/EcalPedestalHistory.cc:DEFINE_FWK_MODULE( EcalPedestalHistory );

@smuzaffar
Copy link
Contributor

#25509 should fix the lost dictionary issue.

@perrotta
Copy link
Contributor

Thank you @smuzaffar

@perrotta
Copy link
Contributor

+1

  • Pixel FED bad channels are included in simulation based on template scenarios stored in the DB
  • The implementation follows what reported in the description
  • Some performance or reliability improvement still possible, but given the temporary nature foreseen for this PR, which is expected to be reverted as soon as RelVals with these bad channels can be produced and studied, we did not insist for having improvements here
  • Default behavior (which applies whenever the new bad pixels payloads are not in the DB) is unchanged with respect to the baseline: jenkins tests pass and show no differences

@fabiocos
Copy link
Contributor

@civanch @tocheng @lpernie @pohsun what is the status of your review?

@civanch
Copy link
Contributor

civanch commented Dec 18, 2018

+1

@pohsun
Copy link

pohsun commented Dec 18, 2018

+1

@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

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