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
SimDataFormats/ValidationFormats - move G4 dependency out of data format #31273
Conversation
… it (which already had a G4 dependency
The code-checks are being triggered in jenkins. |
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31273/17961
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31273/17962
|
A new Pull Request was created by @davidlange6 (David Lange) for master. It involves the following packages: BigProducts/Simulation @smuzaffar, @civanch, @Dr15Jones, @makortel, @mdhildreth, @cmsbuild can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @davidlange6. Is it possible for you to update the same producer in the dd4hep
folder please?
if (enter_sensitive) { | ||
if (m_track_volume != nullptr) { | ||
edm::LogWarning("TrackingMaterialProducer") << "Entering volume " << sensitive << "while inside volume " | ||
<< m_track_volume << ". Something is inconsistent"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @davidlange6. If this is inconsistent, is a warning enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No idea. it was a cerr which I wanted to make a message logger
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davidlange6 by running:
cmsRun SimTracker/TrackerMaterialAnalysis/test/trackingMaterialProducer10GeVNeutrino_ForPhaseI.py
Now I get thousands of printouts like this:
Entering volume TECModule2RphiActivewhile inside volume TECModule1StereoActive. Something is inconsistent
Entering volume TECModule1RphiActivewhile inside volume TECModule2RphiActive. Something is inconsistent
Entering volume TECModule1StereoActivewhile inside volume TECModule1RphiActive. Something is inconsistent
Entering volume TECModule2RphiActivewhile inside volume TECModule1StereoActive. Something is inconsistent
Entering volume TECModule2RphiActivewhile inside volume TECModule2RphiActive. Something is inconsistent
Entering volume TECModule3RphiActivewhile inside volume TECModule2RphiActive. Something is inconsistent
Entering volume TECModule3RphiActivewhile inside volume TECModule3RphiActive. Something is inconsistent
Entering volume TECModule4StereoActivewhile inside volume TECModule3RphiActive. Something is inconsistent
Entering volume TECModule4RphiActivewhile inside volume TECModule4StereoActive. Something is inconsistent
Entering volume TECModule3RphiActivewhile inside volume TECModule4RphiActive. Something is inconsistent
Entering volume TECModule4StereoActivewhile inside volume TECModule3RphiActive. Something is inconsistent
Entering volume TECModule4RphiActivewhile inside volume TECModule4StereoActive. Something is inconsistent
Entering volume PixelBarrelActiveFull1while inside volume PixelBarrelActiveFull0. Something is inconsistent
Entering volume PixelBarrelActiveFull2while inside volume PixelBarrelActiveFull1. Something is inconsistent
Entering volume PixelBarrelActiveFull3while inside volume PixelBarrelActiveFull2. Something is inconsistent
Entering volume TIBActiveRphi0while inside volume PixelBarrelActiveFull3. Something is inconsistent
Entering volume TIBActiveSter0while inside volume TIBActiveRphi0. Something is inconsistent
Entering volume TIBActiveRphi0while inside volume TIBActiveSter0. Something is inconsistent
Entering volume TIBActiveSter0while inside volume TIBActiveRphi0. Something is inconsistent
Entering volume TIBActiveRphi2while inside volume TIBActiveSter0. Something is inconsistent
Entering volume TIBActiveRphi2while inside volume TIBActiveRphi2. Something is inconsistent
Entering volume TOBActiveRphi0while inside volume TIBActiveRphi2. Something is inconsistent
Entering volume TOBActiveRphi0while inside volume TOBActiveRphi0. Something is inconsistent
Entering volume TOBActiveSter0while inside volume TOBActiveRphi0. Something is inconsistent
Entering volume TOBActiveRphi0while inside volume TOBActiveSter0. Something is inconsistent
Entering volume TOBActiveRphi2while inside volume TOBActiveRphi0. Something is inconsistent
Entering volume TOBActiveRphi2while inside volume TOBActiveRphi2. Something is inconsistent
Entering volume PixelForwardSensorwhile inside volume PixelBarrelActiveFull0. Something is inconsistent
Entering volume PixelForwardSensorwhile inside volume PixelForwardSensor. Something is inconsistent
Entering volume PixelForwardSensorwhile inside volume PixelForwardSensor. Something is inconsistent
Entering volume PixelForwardSensorwhile inside volume PixelForwardSensor. Something is inconsistent
Entering volume PixelBarrelActiveFull1while inside volume PixelBarrelActiveFull0. Something is inconsistent
Entering volume PixelBarrelActiveFull2while inside volume PixelBarrelActiveFull1. Something is inconsistent
Can you please double check that these changes do not change the expected behavior? as I pointed out before merging histograms from the unit test are also empty. Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in #31752
if (leave_sensitive) { | ||
if (m_track_volume != sensitive) { | ||
edm::LogWarning("TrackingMaterialProducer") << "Leaving volume " << sensitive << "while inside volume " | ||
<< m_track_volume << ". Something is inconsistent"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be an error/exception?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It wasn't before. No change in behavior is proposed...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, but this was handled internally by SimDataFormats/ValidationFormats/src/MaterialAccountingTrack.cc
. I get a couple of empty histograms. FYI: @mmusich, @mtosi, @apsallid
can you be more specific? I don't see any needed change. |
Thank you @davidlange6 Please disregard my previous comment, this particular file didn't use any of the |
please test |
The tests are being triggered in jenkins.
|
+1 |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
It’s a condition that doesn’t happen. I’ve handled the error as it was done before unless ice misunderstood the code
Cheers,
David
On 28 Aug 2020, at 17:20, Andres Vargas <notifications@github.com> wrote:
@vargasa commented on this pull request.
________________________________
In SimTracker/TrackerMaterialAnalysis/plugins/TrackingMaterialProducer.cc<#31273 (comment)>:
m_track.step(MaterialAccountingStep(length, radiationLengths, energyLoss, globalPositionIn, globalPositionOut));
- if (leave_sensitive)
- m_track.leaveDetector(sensitive, cosThetaPost);
-
+ if (leave_sensitive) {
+ if (m_track_volume != sensitive) {
+ edm::LogWarning("TrackingMaterialProducer") << "Leaving volume " << sensitive << "while inside volume "
+ << m_track_volume << ". Something is inconsistent";
I see, but this was handled internally by SimDataFormats/ValidationFormats/src/MaterialAccountingTrack.cc. I get a couple of empty histograms<https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-961ef0/8982/trackerMaterialAnalysisPlots/>. FYI: @mmusich<https://github.com/mmusich>, @mtosi<https://github.com/mtosi>, @apsallid<https://github.com/apsallid>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#31273 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABGPFQYVDBHGPPSRPGFPEYDSC7DI7ANCNFSM4QNZCPNA>.
|
+1 |
@makortel - any feedback on this PR? |
+core |
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. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
Moved into TrackingMaterialProducer which already depends on G4.