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

fixes for AddOn Test Failures #23019

Merged
merged 2 commits into from
Apr 23, 2018

Conversation

skurz
Copy link
Contributor

@skurz skurz commented Apr 20, 2018

Adressed two issues discussed in #22721 that could lead to rare failures in case of large production samples.

-> Cast in HelixTrajectory did not work, so now explicitly included CopyConstructor
-> HepPDT::ParticleData does not contain information about some high mass resonances that are sometimes produced by the pythia decayer even though they should be according to [2]. They have short lifetimes so decay them again right away.

[2] http://pdg.lbl.gov/2007/reviews/montecarlorpp.pdf

@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 @skurz for master.

It involves the following packages:

FastSimulation/SimplifiedGeometryPropagator

@cmsbuild, @ssekmen, @lveldere, @mdhildreth, @civanch can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @matt-komm 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

@ssekmen
Copy link
Contributor

ssekmen commented Apr 20, 2018

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 20, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/27583/console Started: 2018/04/20 13:49

// in very few events the Decayer (pythia) produces high mass resonances that are for some reason not present in the table (even though they should technically be)
// they have short lifetimes, so decay them right away (charge and lifetime cannot be taken from table)
particle->setRemainingProperLifeTimeC(0.);
particle->setCharge(0.);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skurz @ssekmen are these sufficient protections to get rid of the issue? Which particles are causing this in a standard model run, and why are there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So far we've observed particles with pdgIDs 9010221 and 30443. They can be produced if a non-stable particle is decayed https://github.com/skurz/cmssw/blob/157e9a58bcfcc99d62a9744e34edec23d5f0b2ab/FastSimulation/SimplifiedGeometryPropagator/src/Decayer.cc#L33

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For an initial test of 6000 events, there was no crash with the fix (whereas the current version crashes).

@@ -147,7 +147,8 @@ double fastsim::HelixTrajectory::nextCrossingTimeC(const BarrelSimplifiedGeometr
if(std::abs(layer.getRadius() - getRadParticle(phi1)) > 1.0e-2
|| std::abs(layer.getRadius() - getRadParticle(phi2)) > 1.0e-2)
{
return ((StraightTrajectory*) this)->nextCrossingTimeC(layer, onLayer);
StraightTrajectory traj(*((Trajectory*) this));
Copy link
Contributor

@Dr15Jones Dr15Jones Apr 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cast is unnecessary.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@kpedro88
Copy link
Contributor

I ran a few tests as well:

  • 2000 events in single-threaded mode with no hanging observed
  • 10000 events in multithreaded mode with no exceptions

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23019/27583/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 29
  • DQMHistoTests: Total histograms compared: 2492830
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2492653
  • DQMHistoTests: Total skipped: 176
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 23 files compared)
  • Checked 119 log files, 9 edm output root files, 29 DQM output files

@fabiocos
Copy link
Contributor

@skurz @ssekmen could you please look at @Dr15Jones comment and then confirm if this PR is ok for you? It would be good to have it into 10_2_0_pre2 at the beginning of next week

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

@skurz
Copy link
Contributor Author

skurz commented Apr 23, 2018

@fabiocos I removed the cast according to the comment, so the PR should be ready for sign off.

@cmsbuild
Copy link
Contributor

Pull request #23019 was updated. @cmsbuild, @ssekmen, @lveldere, @mdhildreth, @civanch can you please check and sign again.

@ssekmen
Copy link
Contributor

ssekmen commented Apr 23, 2018

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 23, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/27607/console Started: 2018/04/23 10:07

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-23019/27607/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 29
  • DQMHistoTests: Total histograms compared: 2492830
  • DQMHistoTests: Total failures: 1
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2492653
  • DQMHistoTests: Total skipped: 176
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 28 files compared)
  • Checked 119 log files, 9 edm output root files, 29 DQM output files

@ssekmen
Copy link
Contributor

ssekmen commented Apr 23, 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.

6 participants