Modifications of the Pixel Charge Reweighting for RunDependent MC #37823

carolinecollard · 2022-05-05T16:04:22Z

PR description:

Modifications of the code to allow the Pixel Charge Reweighting at the mixing step in view of future runDependent MC production. The idea is first to produce a premix Library in which no Charge Reweighting is applied, but with stored extra SimHit Information. Then in a second step, during the mixing part, a "Late" Charge Reweighting is applied on the digis from PU, using the extra information. See Last presentation during the AlcaDB meeting for more details.

By default, the standard Charge Reweighting is applied.

To use the new functionality, the runDependent processModifier can be used (but for the moment there is a problem with running this processModifier with the 2018 or 2021 configurations -- AlcaDB people are aware of that).

Some review has already been done within the tracker DPG group.

PR validation:

The standard runTheMatrix workflows should give no differences, because by default we stay with the standard Charge Reweighting approach.

A lot of tests have been performed within the Pixel Offline group to understand the impact of using the Late Charge Reweighthing approach. Comparisons between the two approaches (Standard and Late CR) have been done and differences are understood and expected.

cmsbuild · 2022-05-05T16:13:20Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37823/29738

This PR adds an extra 76KB to repository

cmsbuild · 2022-05-05T16:13:44Z

A new Pull Request was created by @carolinecollard for master.

It involves the following packages:

SimDataFormats/TrackerDigiSimLink (simulation)
SimGeneral/MixingModule (simulation)
SimGeneral/PreMixingModule (simulation)
SimTracker/Common (simulation)
SimTracker/SiPixelDigitizer (simulation)

@cmsbuild, @civanch, @mdhildreth can you please review it and eventually sign? Thanks.
@echabert, @pieterdavid, @robervalwalsh, @CeliaFernandez, @bsunanda, @OzAmram, @threus, @mmusich, @cericeci, @makortel, @JanFSchulte, @jhgoh, @dgulhan, @apsallid, @prolay, @HuguesBrun, @slomeo, @ferencek, @trocino, @sscruz, @abbiendi, @GiacomoSguazzoni, @rovere, @VinInn, @ebrondol, @mtosi, @fabiocos, @gbenelli, @Fedespring, @dkotlins, @lecriste, @tvami this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

mmusich · 2022-05-05T16:14:13Z

test parameters:

workflow = 250202.182

mmusich · 2022-05-05T16:14:53Z

please test

tvami · 2022-05-05T16:39:56Z

urgent

this should go into the next pre-release

mmusich · 2022-05-05T16:47:12Z

urgent

may I ask again for the record, why the urge?

tvami · 2022-05-05T16:53:10Z

may I ask again for the record, why the urge?

If this goes into the main release we can do tests with it, my understanding is that McM system doesnt like pre-releases, but maybe I'm wrong there. Alternatively we can let this slip into the next cycle and have a backport but then isnt it just better to set it to urgent and try to put it into this?

tvami · 2022-05-05T16:53:15Z

type trk

mmusich · 2022-05-05T16:59:44Z

If this goes into the main release we can do tests with it,

can't the test be done via relvals? Or is PPD directly targeting a MC events production on large scale?

tvami · 2022-05-05T17:02:06Z

can't the test be done via relvals? Or is PPD directly targeting a MC events production on large scale?

My understanding that at this point we also need input from O&C about DBS related question. I'm not sure if that can be done with the RelVals or we need McM. Let me tag @rappoccio

rappoccio · 2022-05-05T17:08:34Z

The last time we did this, we used relvals. However, full production will require a bit more coordination and complication. If we go with 12.5 (or even later) we could still test this particular functionality with the relvals while we work with O+C on the full set of production tools.

makortel · 2022-05-05T16:31:52Z

SimDataFormats/TrackerDigiSimLink/BuildFile.xml

@@ -1,6 +1,8 @@
 <use name="DataFormats/Common"/>
 <use name="SimDataFormats/EncodedEventId"/>
 <use name="boost"/>
+<use name="DataFormats/GeometryVector"/>
+<use name="root"/>


Why is the dependence on full ROOT needed?

makortel · 2022-05-05T17:41:08Z

SimDataFormats/TrackerDigiSimLink/src/classes_def.xml

@@ -35,4 +35,17 @@
 -->
 <class name="edm::Wrapper< StripCompactDigiSimLinks >"  />

+  <class name="PixelSimHitExtraInfo" ClassVersion="3">
+   <field name="chan"     mapping="blob" />


What is the intent of this line? (the class does not seem to have chan member variable)

@makortel Yes, chan is a member variable of the class. See here.

The history behing these lines : I had a lot of troubles to write the class in the EDM file. The problem was that I saw the new collection within a TBrowser but I was not able to see the content of the chan vector. So at some point in the past, I added the dependence to root in the BuildFile, and also this line in the classes_def.xml. Then later, I realized that I can have accessed to this vector with CMSSW code, but not within an interactive root session. I have never tried to remove these two lines...

"chan" or "chan_"?

cmsbuild · 2022-05-05T17:53:06Z

-1

Failed Tests: RelVals RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e25ca9/24480/summary.html
COMMIT: 70900d5
CMSSW: CMSSW_12_4_X_2022-05-05-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/37823/24480/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

250202.182

----- Begin Fatal Exception 05-May-2022 19:45:17 CEST-----------------------
An exception of category 'PreMixingSiPixelWorker' occurred while
   [0] Processing  Event run: 315257 lumi: 1 event: 1 stream: 0
   [1] Running path 'HLTAnalyzerEndpath'
   [2] Prefetching for module L1TRawToDigi/'hltGtStage2Digis'
   [3] Prefetching for module RawDataCollectorByLabel/'rawDataCollector'
   [4] Prefetching for module SiStripDigiToRawModule/'SiStripDigiToRaw'
   [5] Calling method for module PreMixingModule/'mixData'
Exception Message:
 Problem in accessing the Extra Pixel SimHit Collection for Late Charge Reweighting 
----- End Fatal Exception -------------------------------------------------

RelVals-INPUT

250202.172250202.172_TTbar_13UP17+TTbar_13UP17INPUT+DIGIPRMXUP17_PU25_RD+RECOPRMXUP17_PU25+HARVESTUP17_PU25/step2_TTbar_13UP17+TTbar_13UP17INPUT+DIGIPRMXUP17_PU25_RD+RECOPRMXUP17_PU25+HARVESTUP17_PU25.log

mmusich · 2022-05-05T18:13:46Z

so, the issue with 250202.172 and 250202.182 is that in those workflows the late-reweighting is (rightfully) active, but the needed extra collection (edm::DetSetVector<PixelSimHitExtraInfo>) is not (yet) available in the input PREMIX libraries:

/RelValPREMIXUP17_PU25/CMSSW_10_6_0-PU25ns_106X_mc2017_realistic_v3-v1/PREMIX
/RelValPREMIXUP18_PU25/CMSSW_11_2_0_pre8-PU_112X_upgrade2018_realistic_v4-v1/PREMIX

Not sure how to solve this, as one would need this PR to create a new one.

mmusich · 2022-05-06T07:41:39Z

Not sure how to solve this, as one would need this PR to create a new one.

so, a possible way forward is:

remove the late-charge re-weighting from the runDependent modifier, to let 250202.172 and 250202.182 pass the PR tests;
(re-)introduce a different ad-hoc modifier just for the charge-reweighting;
introduce a new workflow in the matrix that employs the new modifier and that re-creates on the fly the premixing library, (in the same guise of 250202.181);
test that new workflow in this PR;
merge the PR;
create a new PREMIX library with the additions introduced here;
once it's available, go back and substitute the ad-hoc modifier with the runDependent one along with changing the input PREMIX library in the inputs of the RD premixed relvals.

What do people think?

jpata · 2022-05-30T14:53:10Z

unassign reconstruction

as far as I can tell, there are no modifications in reco (feel free to correct me if I missed smth).

mmusich · 2022-05-30T15:21:55Z

as far as I can tell, there are no modifications in reco (feel free to correct me if I missed smth).

This PR was not reconstruction pending to start with.
A wrong rebase triggered a massive round of unnecessary pings, but it should be clean by now.

cmsbuild · 2022-05-30T19:02:02Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e25ca9/25076/summary.html
COMMIT: ea77c7a
CMSSW: CMSSW_12_5_X_2022-05-29-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/37823/25076/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

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

/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-e25ca9/250202.182_TTbar_13UP18_RD+TTbar_13UP18_RD+DIGIPRMXUP18_PU25_RD+RECOPRMXUP18_PU25_RD+HARVESTUP18_PU25_RD
/data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-e25ca9/250202.184_TTbar_13UP18+TTbar_13UP18+PREMIXUP18_PU25_RDPix+DIGIPRMXLOCALUP18_PU25_RDPix+RECOPRMXUP18_PU25+HARVESTUP18_PU25

Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3648315
DQMHistoTests: Total failures: 8
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3648285
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
Checked 208 log files, 45 edm output root files, 50 DQM output files
TriggerResults: no differences found

mmusich · 2022-06-01T06:58:30Z

@cms-sw/simulation-l2 @cms-sw/pdmv-l2 @cms-sw/upgrade-l2
you already signed once this PR, it was changed to comply with the requests from @cms-sw/orp-l2.
Can you please check and sign again?
Thank you.

kskovpen · 2022-06-01T07:01:11Z

+pdmv

srimanob · 2022-06-01T12:18:29Z

+Upgrade
re-sign

civanch · 2022-06-02T10:57:15Z

+1

tvami · 2022-06-02T14:58:39Z

@cms-sw/orp-l2 this is essentially fully signed

perrotta · 2022-06-02T21:37:37Z

SimTracker/SiPixelDigitizer/plugins/SiPixelChargeReweightingAlgorithm.cc

+
+  array_2d pixrewgt(boost::extents[TXSIZE][TYSIZE]);
+
+  /*


I don't want to retrigger another set of corrections and signatures, but this commented out code should be removed, even in a follow up PR

perrotta · 2022-06-02T21:41:22Z

+1

cmsbuild · 2022-06-02T21:41:46Z

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 be automatically merged.

davidlange6 · 2022-10-11T08:10:19Z

Thx so 60kB per event. Not tiny but not so big On May 6, 2022 3:32 PM, carolinecollard ***@***.***> wrote: Thx- seeing tracking particles I suspect the file you used had lots of debug information in it. How many events was that test for? 50 events, with the standart output format. — Reply to this email directly, view it on GitHub<#37823 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABGPFQYFQ3VSDLBEDYS2F73VIUNL3ANCNFSM5VFRLTXA>. You are receiving this because you were mentioned.Message ID: ***@***.***>

cmsbuild added this to the CMSSW_12_4_X milestone May 5, 2022

cmsbuild added code-checks-pending orp-pending pending-signatures simulation-pending tests-pending labels May 5, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels May 5, 2022

cmsbuild added tests-started and removed tests-pending labels May 5, 2022

cmsbuild added the urgent label May 5, 2022

cmsbuild added the trk label May 5, 2022

makortel reviewed May 5, 2022

View reviewed changes

cmsbuild added tests-rejected and removed tests-started labels May 5, 2022

mmusich force-pushed the additionalDigiInfo_from-CMSSW_12_4_0_pre1 branch from 70900d5 to 388d825 Compare May 6, 2022 08:27

cmsbuild added the tests-started label May 30, 2022

cmsbuild added tests-approved and removed tests-started labels May 30, 2022

cmsbuild added pdmv-approved and removed pdmv-pending labels Jun 1, 2022

cmsbuild added upgrade-approved and removed upgrade-pending labels Jun 1, 2022

cmsbuild mentioned this pull request Jun 1, 2022

Shorten the names of two ALCARECOStreams in order to fit into DBS database schema limitations #38186

Merged

cmsbuild added simulation-approved and removed simulation-pending labels Jun 2, 2022

perrotta reviewed Jun 2, 2022

View reviewed changes

cmsbuild added fully-signed operations-approved orp-approved and removed operations-pending pending-signatures orp-pending labels Jun 2, 2022

cmsbuild merged commit 4e30acd into cms-sw:master Jun 2, 2022

qliphy mentioned this pull request Jun 3, 2022

Issues with wfs 25040X in IB tests #38229

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modifications of the Pixel Charge Reweighting for RunDependent MC #37823

Modifications of the Pixel Charge Reweighting for RunDependent MC #37823

carolinecollard commented May 5, 2022

cmsbuild commented May 5, 2022

cmsbuild commented May 5, 2022

mmusich commented May 5, 2022

mmusich commented May 5, 2022

tvami commented May 5, 2022

mmusich commented May 5, 2022

tvami commented May 5, 2022

tvami commented May 5, 2022

mmusich commented May 5, 2022

tvami commented May 5, 2022

rappoccio commented May 5, 2022

makortel May 5, 2022

makortel May 5, 2022

carolinecollard May 6, 2022

davidlange6 May 6, 2022

cmsbuild commented May 5, 2022

mmusich commented May 5, 2022 •

edited

mmusich commented May 6, 2022 •

edited

jpata commented May 30, 2022

mmusich commented May 30, 2022 •

edited

cmsbuild commented May 30, 2022

mmusich commented Jun 1, 2022

kskovpen commented Jun 1, 2022

srimanob commented Jun 1, 2022

civanch commented Jun 2, 2022

tvami commented Jun 2, 2022

perrotta Jun 2, 2022

perrotta commented Jun 2, 2022

cmsbuild commented Jun 2, 2022

davidlange6 commented Oct 11, 2022 via email

Modifications of the Pixel Charge Reweighting for RunDependent MC #37823

Modifications of the Pixel Charge Reweighting for RunDependent MC #37823

Conversation

carolinecollard commented May 5, 2022

PR description:

PR validation:

cmsbuild commented May 5, 2022

cmsbuild commented May 5, 2022

mmusich commented May 5, 2022

mmusich commented May 5, 2022

tvami commented May 5, 2022

mmusich commented May 5, 2022

tvami commented May 5, 2022

tvami commented May 5, 2022

mmusich commented May 5, 2022

tvami commented May 5, 2022

rappoccio commented May 5, 2022

makortel May 5, 2022

Choose a reason for hiding this comment

makortel May 5, 2022

Choose a reason for hiding this comment

carolinecollard May 6, 2022

Choose a reason for hiding this comment

davidlange6 May 6, 2022

Choose a reason for hiding this comment

cmsbuild commented May 5, 2022

RelVals

RelVals-INPUT

mmusich commented May 5, 2022 • edited

mmusich commented May 6, 2022 • edited

jpata commented May 30, 2022

mmusich commented May 30, 2022 • edited

cmsbuild commented May 30, 2022

Comparison Summary

mmusich commented Jun 1, 2022

kskovpen commented Jun 1, 2022

srimanob commented Jun 1, 2022

civanch commented Jun 2, 2022

tvami commented Jun 2, 2022

perrotta Jun 2, 2022

Choose a reason for hiding this comment

perrotta commented Jun 2, 2022

cmsbuild commented Jun 2, 2022

davidlange6 commented Oct 11, 2022 via email

mmusich commented May 5, 2022 •

edited

mmusich commented May 6, 2022 •

edited

mmusich commented May 30, 2022 •

edited