Introduce flag for particles generation with max energy spread #36460

lecriste · 2021-12-11T12:59:41Z

PR description:

This PR introduces the option to generate particles with the maximum spread in the energy range specified.
No changes are expected in the output as by default the option is disabled.

PR validation:

Produced a few samples and verified that the generated particle energies are the desired ones.

cmsbuild · 2021-12-11T13:09:18Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36460/27306

This PR adds an extra 32KB to repository

cmsbuild · 2021-12-11T13:09:40Z

A new Pull Request was created by @lecriste (Leonardo Cristella) 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, @jan-kaspar, @fabiocos 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

SiewYan · 2021-12-12T03:38:37Z

please test

SiewYan · 2021-12-12T03:40:39Z

IOMC/ParticleGuns/src/CloseByParticleGunProducer.cc

+    double fEn;
+    if (numParticles > 1 && fMaxEnSpread)
+      fEn = fEnMin + ip * (fEnMax - fEnMin) / (numParticles - 1);
+    else
+      fEn = CLHEP::RandFlat::shoot(engine, fEnMin, fEnMax);
+


can you explain what has been done here?

@SiewYan, in case the newly introduced flag (MaxEnSpread) is true, the energies of the generated particles are set in constant steps of the specified range (fEnMin, fEnMax); otherwise they are computed randomly in the specified range, as in the original code.

thanks, its all look good to me

cmsbuild · 2021-12-12T06:40:40Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3ecfda/21205/summary.html
COMMIT: 9624a57
CMSSW: CMSSW_12_3_X_2021-12-11-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36460/21205/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: 42
DQMHistoTests: Total histograms compared: 3250704
DQMHistoTests: Total failures: 5
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3250676
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 41 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 177 log files, 37 edm output root files, 42 DQM output files
TriggerResults: no differences found

SiewYan · 2021-12-14T11:27:25Z

+1

cmsbuild · 2021-12-14T11:27:50Z

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 · 2021-12-14T13:54:01Z

The update implemented here is fine.

I wonder why in CloseByParticleGunProducer.cc the std::vector<int> particles is needed at all.
What's wrong with removing L65-L70, and replacing L78-L91 with:

  for (unsigned int ip = 0; ip < numParticles; ++ip) {
    if (fOverlapping) {
      fR = CLHEP::RandFlat::shoot(engine, tmpR - fDelta, tmpR + fDelta);
      phi = CLHEP::RandFlat::shoot(engine, tmpPhi - fDelta / fR, tmpPhi + fDelta / fR);
    } else
      phi += fDelta / fR;

    double fEn;
    if (numParticles > 1 && fMaxEnSpread)
      fEn = fEnMin + ip * (fEnMax - fEnMin) / (numParticles - 1);
    else
      fEn = CLHEP::RandFlat::shoot(engine, fEnMin, fEnMax);

    int PartID = CLHEP::RandFlat::shoot(engine, 0, fPartIDs.size());

cmsbuild · 2021-12-14T19:38:28Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36460/27373

This PR adds an extra 12KB to repository

cmsbuild · 2021-12-14T19:38:56Z

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

SiewYan · 2021-12-15T02:28:39Z

please test

cmsbuild · 2021-12-15T05:30:17Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3ecfda/21280/summary.html
COMMIT: 3e2e47a
CMSSW: CMSSW_12_3_X_2021-12-14-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36460/21280/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: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 42
DQMHistoTests: Total histograms compared: 3250719
DQMHistoTests: Total failures: 0
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3250697
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 41 files compared)
Checked 177 log files, 37 edm output root files, 42 DQM output files
TriggerResults: no differences found

SiewYan · 2021-12-15T05:46:00Z

+1

cmsbuild · 2021-12-15T05:46:24Z

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 · 2021-12-15T07:02:04Z

+1

makortel · 2022-01-25T19:41:19Z

IOMC/ParticleGuns/src/CloseByParticleGunProducer.cc

+    int PartID = CLHEP::RandFlat::shoot(engine, 0, fPartIDs.size());
    const HepPDT::ParticleData* PData = fPDGTable->particle(HepPDT::ParticleID(abs(PartID)));
    double mass = PData->mass().value();


This piece appears to be causing segfaults in 12_3_0_pre2 RelVals, see #36800.

lecriste added 3 commits December 10, 2021 01:33

Add MaxSpread flag

ede9210

Generalize to N particles

383e594

Improve code readability

9624a57

cmsbuild added this to the CMSSW_12_3_X milestone Dec 11, 2021

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

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

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

SiewYan reviewed Dec 12, 2021

View reviewed changes

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

cmsbuild added fully-signed generators-approved and removed generators-pending pending-signatures labels Dec 14, 2021

Remove unnecessary vector

3e2e47a

cmsbuild removed generators-approved fully-signed labels Dec 14, 2021

cmsbuild added code-checks-pending generators-pending pending-signatures tests-pending and removed tests-approved code-checks-approved labels Dec 14, 2021

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

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

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

cmsbuild added fully-signed generators-approved and removed generators-pending pending-signatures labels Dec 15, 2021

cmsbuild added orp-approved and removed orp-pending labels Dec 15, 2021

cmsbuild merged commit b200e08 into cms-sw:master Dec 15, 2021

waredjeb mentioned this pull request Dec 20, 2021

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

Merged

makortel mentioned this pull request Jan 25, 2022

CloseByPGun Samples Failure #36800

Closed

makortel reviewed Jan 25, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce flag for particles generation with max energy spread #36460

Introduce flag for particles generation with max energy spread #36460

lecriste commented Dec 11, 2021

cmsbuild commented Dec 11, 2021

cmsbuild commented Dec 11, 2021

SiewYan commented Dec 12, 2021

SiewYan Dec 12, 2021

lecriste Dec 14, 2021 •

edited

Loading

SiewYan Dec 14, 2021

cmsbuild commented Dec 12, 2021

SiewYan commented Dec 14, 2021

cmsbuild commented Dec 14, 2021

perrotta commented Dec 14, 2021

cmsbuild commented Dec 14, 2021

cmsbuild commented Dec 14, 2021

SiewYan commented Dec 15, 2021

cmsbuild commented Dec 15, 2021

SiewYan commented Dec 15, 2021

cmsbuild commented Dec 15, 2021

perrotta commented Dec 15, 2021

makortel Jan 25, 2022

Introduce flag for particles generation with max energy spread #36460

Introduce flag for particles generation with max energy spread #36460

Conversation

lecriste commented Dec 11, 2021

PR description:

PR validation:

cmsbuild commented Dec 11, 2021

cmsbuild commented Dec 11, 2021

SiewYan commented Dec 12, 2021

SiewYan Dec 12, 2021

Choose a reason for hiding this comment

lecriste Dec 14, 2021 • edited Loading

Choose a reason for hiding this comment

SiewYan Dec 14, 2021

Choose a reason for hiding this comment

cmsbuild commented Dec 12, 2021

Comparison Summary

SiewYan commented Dec 14, 2021

cmsbuild commented Dec 14, 2021

perrotta commented Dec 14, 2021

cmsbuild commented Dec 14, 2021

cmsbuild commented Dec 14, 2021

SiewYan commented Dec 15, 2021

cmsbuild commented Dec 15, 2021

Comparison Summary

SiewYan commented Dec 15, 2021

cmsbuild commented Dec 15, 2021

perrotta commented Dec 15, 2021

makortel Jan 25, 2022

Choose a reason for hiding this comment

lecriste Dec 14, 2021 •

edited

Loading