[HGCAL] Closeby Particle gun - uniform eta generation #36554

waredjeb · 2021-12-20T14:33:13Z

This PR introduces the possibility to generate particles with a uniform eta distribution.
if the ControlledByEta flag is True, eta is generated in the range [MinEta,MaxEta] and R is computed from eta. Otherwise, the generation is controlled by R and R is sampled in the range [RMin, RMax]
this PR also fixes a bug introduced by Introduce flag for particles generation with max energy spread #36460
- https://github.com/cms-sw/cmssw/pull/36460/files#diff-40127cfb5d2379d718d6b82ffc0d5e900b6eeb1bae875f44fa81f63655e06365R84 PartID should be taken randomly from fPartIDs, instead was sampled between [0, fPardIDs.size()].

cmsbuild · 2021-12-20T14:42:14Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36554/27492

This PR adds an extra 32KB to repository

cmsbuild · 2021-12-20T14:42:47Z

A new Pull Request was created by @waredjeb (Wahid Redjeb) for master.

It involves the following packages:

Configuration/Generator (generators)
IOMC/ParticleGuns (generators)

@SiewYan, @mkirsano, @Saptaparna, @cmsbuild, @GurpreetSinghChahal, @agrohsje, @alberto-sanchez can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @missirol, @fabiocos, @grzanka 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

rovere · 2021-12-20T14:43:13Z

@cmsbuild please test

cmsbuild · 2021-12-20T18:10:32Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bd6456/21406/summary.html
COMMIT: ae1f104
CMSSW: CMSSW_12_3_X_2021-12-20-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36554/21406/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 6 differences found in the comparisons
DQMHistoTests: Total files compared: 43
DQMHistoTests: Total histograms compared: 3461688
DQMHistoTests: Total failures: 11
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3461654
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 42 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 181 log files, 42 edm output root files, 43 DQM output files
TriggerResults: no differences found

apsallid · 2021-12-22T14:10:50Z

IOMC/ParticleGuns/src/CloseByParticleGunProducer.cc

  fEnMax = pgun_params.getParameter<double>("EnMax");
  fEnMin = pgun_params.getParameter<double>("EnMin");
  fMaxEnSpread = pgun_params.getParameter<bool>("MaxEnSpread");
+  fEtaMax = pgun_params.getParameter<double>("MinEta");
+  fEtaMin = pgun_params.getParameter<double>("MaxEta");


@waredjeb Min is max and max is min here. Maybe they should be flipped? Also, could you please add a line in the description about the bug that PR #36460 introduced?

Thanks! I fixed the names and added a description of the bug introduced

perrotta

Thank you @waredjeb for fixing a bug that was originally introduced from a comment of mine in #36460
Since you are cleaning, could you please consider also the following suggestions?

perrotta · 2021-12-27T09:47:53Z

IOMC/ParticleGuns/src/CloseByParticleGunProducer.cc

+  if (!fControlledByEta) {
+    fR = CLHEP::RandFlat::shoot(engine, fRMin, fRMax);
+  } else {
+    fEta = CLHEP::RandFlat::shoot(engine, fEtaMin, fEtaMax);


Not fundamental, but fEta could have been more conveniently defined as a local variable only inside this scope...

perrotta · 2021-12-27T10:01:14Z

IOMC/ParticleGuns/src/CloseByParticleGunProducer.cc

  fEnMax = pgun_params.getParameter<double>("EnMax");
  fEnMin = pgun_params.getParameter<double>("EnMin");
  fMaxEnSpread = pgun_params.getParameter<bool>("MaxEnSpread");
+  fEtaMax = pgun_params.getParameter<double>("MaxEta");


Since you are (also) cleaning, the same style could be used for all Min/Max parameters: here EtaMin / EtaMax (and below also PhiMin / PhiMax)

Hi! I tried to change from MinEta/MaxEta to EtaMin/EtaMax, I modified also the _cfi.py files, but when I try to test the workflow, I get

----- Begin Fatal Exception 28-Dec-2021 13:07:11 CET----------------------- An exception of category 'Configuration' occurred while [0] Constructing the EventProcessor [1] Constructing module: class=CloseByParticleGunProducer label='generator' Exception Message: MissingParameter: Parameter 'MinEta' not found. ----- End Fatal Exception -------------------------------------------------

Same thing with Phi.
Am I missing something?

@waredjeb the message says that it looks from a "MinEta" parameter which is not able to find in the configuration. This makes me think that you did not modify in your patch the fillDescriptions method with the new parameter' names

@perrotta The point is that there is not a fillDescription in the CloseByParticleGunProducer, that's why I modified the cfi.py manually

Ah, ok. Then you did not modify the pgun_params.getParameter<double>("MinEta") call in the .cc code (or you did it but you did not recompile before testing)

I found the problem, I had to change the parameter name in IOMC/ParticleGuns/src/BaseFlatGunProducer.cc as well.

perrotta · 2021-12-27T10:08:02Z

IOMC/ParticleGuns/src/CloseByParticleGunProducer.cc

@@ -25,9 +25,12 @@ CloseByParticleGunProducer::CloseByParticleGunProducer(const ParameterSet& pset)
  ParameterSet defpset;


This defpset doesn't look to be used anywhere, and it can be removed

perrotta · 2021-12-27T10:13:12Z

IOMC/ParticleGuns/src/CloseByParticleGunProducer.cc

+  fEtaMax = pgun_params.getParameter<double>("MaxEta");
+  fEtaMin = pgun_params.getParameter<double>("MinEta");
  fRMax = pgun_params.getParameter<double>("RMax");
  fRMin = pgun_params.getParameter<double>("RMin");


Suggested change

fEtaMax = pgun_params.getParameter<double>("MaxEta");

fEtaMin = pgun_params.getParameter<double>("MinEta");

fRMax = pgun_params.getParameter<double>("RMax");

fRMin = pgun_params.getParameter<double>("RMin");

if (fControlledByEta) {

fEtaMax = pgun_params.getParameter<double>("EtaMax");

fEtaMin = pgun_params.getParameter<double>("EtaMin");

if (fEtaMax <= fEtaMin)

LogError("CloseByParticleGunProducer") << " Please fix EtaMin and EtaMax values in the configuration";

} else {

fRMax = pgun_params.getParameter<double>("RMax");

fRMin = pgun_params.getParameter<double>("RMin");

if (fRMax <= fRMin)

LogError("CloseByParticleGunProducer") << " Please fix RMin and RMax values in the configuration";

}

could avoid mistakes while configuring

Please also remove the useless << endl at the end of the two LogDebug statements in this code

cmsbuild · 2021-12-28T13:58:54Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36554/27580

This PR adds an extra 36KB to repository

cmsbuild · 2021-12-28T13:59:12Z

Pull request #36554 was updated. @SiewYan, @mkirsano, @Saptaparna, @cmsbuild, @GurpreetSinghChahal, @agrohsje, @alberto-sanchez can you please check and sign again.

perrotta · 2021-12-28T17:14:18Z

please test

cmsbuild · 2021-12-28T19:55:55Z

-1

Failed Tests: RelVals RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bd6456/21491/summary.html
COMMIT: af35508
CMSSW: CMSSW_12_3_X_2021-12-28-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36554/21491/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

250202.181

----- Begin Fatal Exception 28-Dec-2021 20:26:01 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=FlatRandomEGunProducer label='generator'
Exception Message:
MissingParameter: Parameter 'EtaMin' not found.
----- End Fatal Exception -------------------------------------------------

35234.999

----- Begin Fatal Exception 28-Dec-2021 20:29:43 CET-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=FlatRandomEGunProducer label='generator'
Exception Message:
MissingParameter: Parameter 'EtaMin' not found.
----- End Fatal Exception -------------------------------------------------

RelVals-INPUT

11834.9911834.99_TTbar_14TeV+2021PU_PMXS1S2+TTbar_14TeV_TuneCP5_GenSimINPUT+PREMIX_PremixPU+DigiPU+RecoNanoPU+HARVESTNanoPU/step2_TTbar_14TeV+2021PU_PMXS1S2+TTbar_14TeV_TuneCP5_GenSimINPUT+PREMIX_PremixPU+DigiPU+RecoNanoPU+HARVESTNanoPU.log
12634.9912634.99_TTbar_14TeV+2023PU_PMXS1S2+TTbar_14TeV_TuneCP5_GenSimINPUT+PREMIX_PremixPU+DigiPU+RecoNanoPU+HARVESTNanoPU/step2_TTbar_14TeV+2023PU_PMXS1S2+TTbar_14TeV_TuneCP5_GenSimINPUT+PREMIX_PremixPU+DigiPU+RecoNanoPU+HARVESTNanoPU.log
13034.9913034.99_TTbar_14TeV+2024PU_PMXS1S2+TTbar_14TeV_TuneCP5_GenSimINPUT+PREMIX_PremixPU+DigiPU+RecoNanoPU+HARVESTNanoPU/step2_TTbar_14TeV+2024PU_PMXS1S2+TTbar_14TeV_TuneCP5_GenSimINPUT+PREMIX_PremixPU+DigiPU+RecoNanoPU+HARVESTNanoPU.log

Expand to see more relval errors ...

cmsbuild · 2022-01-14T21:59:48Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bd6456/21736/summary.html
COMMIT: b28d61a
CMSSW: CMSSW_12_3_X_2022-01-14-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36554/21736/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 43
DQMHistoTests: Total histograms compared: 3461659
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3461631
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 42 files compared)
Checked 181 log files, 42 edm output root files, 43 DQM output files
TriggerResults: no differences found

rovere · 2022-01-17T17:16:47Z

Since this PR is modifying the behaviour of the CloseByParticleGun, it would be nice to have this tested with some workflow actually using that generator snippet.
Any authoritative suggestion for how to properly do that?

rovere · 2022-01-19T13:16:10Z

ping....

SiewYan · 2022-01-24T12:17:09Z

Hello @rovere , I am not sure how to trigger user specific workflow on the PR interface. @perrotta do you know the command? I cannot find it in the cmssw documentation.

rovere · 2022-01-27T15:04:18Z

test parameters:

workflow = 38696
relvals_opt = --what upgrade,standard,highstats,pileup,generator,extendedgen,production,ged,machine,premix

rovere · 2022-01-27T15:04:56Z

@cmsbuild please test

cmsbuild · 2022-01-27T19:09:31Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bd6456/22049/summary.html
COMMIT: b28d61a
CMSSW: CMSSW_12_3_X_2022-01-26-2300/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36554/22049/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-bd6456/38696.0_CloseByPGun_CE_E_Front_120um+2026D86+CE_E_Front_120um_GenSimHLBeamSpotHGCALCloseBy+DigiTrigger+RecoGlobal+HARVESTGlobal

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 43
DQMHistoTests: Total histograms compared: 3449324
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3449302
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 42 files compared)
Checked 181 log files, 42 edm output root files, 43 DQM output files
TriggerResults: no differences found

rovere · 2022-01-27T20:19:40Z

The special workflow 38696 is using the CloseByPGun generator. It ran fine.
From my side, I have no further check I'd like to perform.
It would be nice to have this PR integrated ASAP.

SiewYan · 2022-01-28T06:16:21Z

+1

@rovere , thank you for the test. I looked into the log file, its fine by me.

cmsbuild · 2022-01-28T06:16: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 now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

perrotta · 2022-01-28T07:57:22Z

+1

cmsbuild added this to the CMSSW_12_3_X milestone Dec 20, 2021

cmsbuild added code-checks-pending generators-pending orp-pending pending-signatures tests-pending labels Dec 20, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 20, 2021

cmsbuild added tests-started and removed tests-pending labels Dec 20, 2021

cmsbuild added tests-approved and removed tests-started labels Dec 20, 2021

apsallid reviewed Dec 22, 2021

View reviewed changes

perrotta reviewed Dec 27, 2021

View reviewed changes

cmsbuild added code-checks-pending tests-pending and removed tests-approved code-checks-approved labels Dec 28, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 28, 2021

cmsbuild added tests-started and removed tests-pending labels Dec 28, 2021

cmsbuild added tests-started and removed tests-pending labels Jan 14, 2022

cmsbuild added tests-approved and removed tests-started labels Jan 14, 2022

bsunanda mentioned this pull request Jan 23, 2022

Run3-gex108XD Modify muon numbering, adding scenarios for the corretced Run3 geometry in CMSSW version 12_2_X #36782

Merged

rovere mentioned this pull request Jan 25, 2022

CloseByPGun Samples Failure #36800

Closed

cmsbuild added tests-started and removed tests-approved labels Jan 27, 2022

cmsbuild added tests-approved and removed tests-started labels Jan 27, 2022

cmsbuild added fully-signed generators-approved and removed generators-pending pending-signatures labels Jan 28, 2022

cmsbuild added orp-approved and removed orp-pending labels Jan 28, 2022

cmsbuild merged commit 789ec14 into cms-sw:master Jan 28, 2022

This was referenced Jan 28, 2022

[AArch64] build cmssw aarch64 with -mno-outline-atomics file cms-sw/cmsdist#7582

Closed

[Aarch64] Build with -march=armv8-a cms-sw/cmsdist#7593

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HGCAL] Closeby Particle gun - uniform eta generation #36554

[HGCAL] Closeby Particle gun - uniform eta generation #36554

waredjeb commented Dec 20, 2021 •

edited

cmsbuild commented Dec 20, 2021

cmsbuild commented Dec 20, 2021

rovere commented Dec 20, 2021

cmsbuild commented Dec 20, 2021

apsallid Dec 22, 2021

waredjeb Dec 27, 2021

perrotta left a comment

perrotta Dec 27, 2021 •

edited

perrotta Dec 27, 2021

waredjeb Dec 28, 2021 •

edited

perrotta Dec 28, 2021

waredjeb Dec 28, 2021

perrotta Dec 28, 2021

waredjeb Dec 28, 2021

perrotta Dec 27, 2021

perrotta Dec 27, 2021 •

edited

perrotta Dec 27, 2021

cmsbuild commented Dec 28, 2021

cmsbuild commented Dec 28, 2021

perrotta commented Dec 28, 2021

cmsbuild commented Dec 28, 2021

cmsbuild commented Jan 14, 2022

rovere commented Jan 17, 2022

rovere commented Jan 19, 2022

SiewYan commented Jan 24, 2022

rovere commented Jan 27, 2022

rovere commented Jan 27, 2022

cmsbuild commented Jan 27, 2022

rovere commented Jan 27, 2022

SiewYan commented Jan 28, 2022

cmsbuild commented Jan 28, 2022

perrotta commented Jan 28, 2022

		@@ -25,9 +25,12 @@ CloseByParticleGunProducer::CloseByParticleGunProducer(const ParameterSet& pset)
		ParameterSet defpset;

[HGCAL] Closeby Particle gun - uniform eta generation #36554

[HGCAL] Closeby Particle gun - uniform eta generation #36554

Conversation

waredjeb commented Dec 20, 2021 • edited

cmsbuild commented Dec 20, 2021

cmsbuild commented Dec 20, 2021

rovere commented Dec 20, 2021

cmsbuild commented Dec 20, 2021

Comparison Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

perrotta left a comment

Choose a reason for hiding this comment

perrotta Dec 27, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

waredjeb Dec 28, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

perrotta Dec 27, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmsbuild commented Dec 28, 2021

cmsbuild commented Dec 28, 2021

perrotta commented Dec 28, 2021

cmsbuild commented Dec 28, 2021

RelVals

RelVals-INPUT

cmsbuild commented Jan 14, 2022

Comparison Summary

rovere commented Jan 17, 2022

rovere commented Jan 19, 2022

SiewYan commented Jan 24, 2022

rovere commented Jan 27, 2022

rovere commented Jan 27, 2022

cmsbuild commented Jan 27, 2022

Comparison Summary

rovere commented Jan 27, 2022

SiewYan commented Jan 28, 2022

cmsbuild commented Jan 28, 2022

perrotta commented Jan 28, 2022

waredjeb commented Dec 20, 2021 •

edited

perrotta Dec 27, 2021 •

edited

waredjeb Dec 28, 2021 •

edited

perrotta Dec 27, 2021 •

edited