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
Run2-TB32 Add simulation for the Timing layer TB studies #19419
Conversation
@cmsbuild Please test |
The tests are being triggered in jenkins. |
A new Pull Request was created by @bsunanda for master. It involves the following packages: Geometry/HGCalCommonData @civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @kpedro88, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
+1 |
+1 |
@@ -0,0 +1,350 @@ | |||
// system include files |
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.
please fix the name of this file (Timin -> Timing)
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.
Done
} else { | ||
const HepMC::GenEvent * myGenEvent = evtMC->GetEvent(); | ||
unsigned int k(0); | ||
for (HepMC::GenEvent::particle_const_iterator p = myGenEvent->particles_begin(); |
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.
If this loop only exists to print debug information, the entire loop can be put inside the EDM_ML_DEBUG
flag (actually, it looks like everything in this else
block can be)
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 is left for future development. We leave this code within the EDM_ML_DEBUG for the time being
|
||
void HGCalTimingAnalyzer::fillDescriptions(edm::ConfigurationDescriptions& descriptions) { | ||
edm::ParameterSetDescription desc; | ||
desc.setUnknown(); |
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.
remove this line (the desc is not unknown)
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.
done
detectorBeam_= iConfig.getParameter<std::string>("DetectorBeam"); | ||
groupHits_ = iConfig.getParameter<bool>("GroupHits"); | ||
timeUnit_ = iConfig.getParameter<double>("TimeUnit"); | ||
idBeams_ = iConfig.getParameter<std::vector<int>>("IDBeams"); |
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.
what is this parameter used for?
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.
Try to add comment
The tests are being triggered in jenkins. |
Pull request #19419 was updated. @civanch, @Dr15Jones, @ianna, @mdhildreth, @cmsbuild, @kpedro88, @davidlange6 can you please check and sign again. |
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
Comparison job queued. |
Comparison is ready @slava77 comparisons for the following workflows were not done due to missing matrix map:
Comparison Summary:
|
+1 |
+1 |
@civanch Could you approve this again? |
+1 |
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 requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar |
merge |
No description provided.