cms-externals / geant4

CMS mirror of Geant4 sources. Not endorsed by Geant4 organization itself under any circumstance.
Other
2 stars 6 forks source link

G4TransportationWithMsc: Always update momentum direction #76

Closed hahnjo closed 1 year ago

hahnjo commented 1 year ago

This fixes a very rare bug when G4Transportation runs without field, G4TransportationWithMsc is allowed to make multiple internal steps, and the last internal step does not sample scattering (because the track is either ranging out, or the step is very small). In this case G4Transportation will reset fMomentumChanged = false because linear propagation does not change the momentum direction, when in fact earlier internal steps updated the direction due to scattering.

Instead always update the momentum direction because the above is rare. Numerical differences are observed in the simulation histories, but not statistically significant.

cmsbuild commented 1 year ago

A new Pull Request was created by @hahnjo (Jonas Hahnfeld) for branch cms/v11.1.2.

@smuzaffar, @iarspider, @aandvalenzuela, @cmsbuild can you please review it and eventually sign? Thanks. @sextonkennedy, @rappoccio, @antoniovilela you are the release manager for this. cms-bot commands are listed here

smuzaffar commented 1 year ago

please test

cmsbuild commented 1 year ago

-1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bbaa5c/35405/summary.html COMMIT: 24bfcac11a1c0c83844d400c4bbe7ddb824515fa CMSSW: CMSSW_13_3_X_2023-10-24-2300/el8_amd64_gcc12 User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-externals/geant4/76/35405/install.sh to create a dev area with all the needed externals and cmssw changes.

cmsbuild commented 1 year ago

-1

Failed Tests: UnitTests Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bbaa5c/35414/summary.html COMMIT: 24bfcac11a1c0c83844d400c4bbe7ddb824515fa CMSSW: CMSSW_13_3_X_2023-10-24-2300/el8_amd64_gcc12 User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-externals/geant4/76/35414/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bbaa5c/35414/git-recent-commits.json https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bbaa5c/35414/git-merge-result

Unit Tests

I found 1 errors in the following unit tests:

---> test testTauEmbeddingProducers had ERRORS

Comparison Summary

Summary:

civanch commented 1 year ago

This PR has nothing to do with the failed unit test. Regression for standard tests is seen as expected. Should tests be resubmitted?

smuzaffar commented 1 year ago

@civanch , are you happy with this change?

smuzaffar commented 1 year ago

+externals

lets get this in and update cmsdist/geant4.spec

cmsbuild commented 1 year ago

This pull request is fully signed and it will be integrated in one of the next cms/v11.1.2 IBs (but tests are reportedly failing). This pull request will now be reviewed by the release team before it's merged. @antoniovilela, @rappoccio, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2)