-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Updating Phase2 Tracker Digitizer validation code #18687
Updating Phase2 Tracker Digitizer validation code #18687
Conversation
…ctor local origin ( Phase2TrackerMonitorDigi ) , correct interpretation of ProcessId ( Phase2TrackerValidateDigi ) , changes in histogram range and addition of a few new histograms. Updated Beamtest analysis code
A new Pull Request was created by @suchandradutta (Suchandra Dutta) for master. It involves the following packages: SimTracker/SiPhase2Digitizer @cmsbuild, @civanch, @mdhildreth, @kpedro88, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
XYOccupancyMap->Fill(gp.x()*10., gp.y()*10, occupancy); | ||
RZOccupancyMap->Fill(gp.z()*10., std::hypot(gp.x(),gp.y())*10., occupancy); | ||
local_mes.EtaOccupancyProfP->Fill(gp.eta(), occupancy); | ||
// if (layer == 1) std::cout << " Det Id = " << rawid << " Eta " << gp.eta() << " r " << std::hypot(gp.x(),gp.y())*10. << " z " << gp.z()*10. << " Occupancy " << occupancy << std::endl; |
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.
Delete commented-out code
@@ -339,7 +340,7 @@ void Phase2TrackerMonitorDigi::bookHistograms(DQMStore::IBooker & ibooker, | |||
ParametersOcc.getParameter<double>("xmax")); | |||
|
|||
Parameters = config_.getParameter<edm::ParameterSet>("RZPositionMapH"); | |||
std::cout << " booking RZ Position " << " R = " << Parameters.getParameter<double>("ymax") << " Z " << Parameters.getParameter<double>("xmin") << " " << Parameters.getParameter<double>("xmax") << std::endl; | |||
std::cout << " booking RZ Position " << " R = " << Parameters.getParameter<double>("ymax") << " Z " << Parameters.getParameter<double>("xmin") << " " << Parameters.getParameter<double>("xmax") << std::endl; |
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.
Use MessageLogger, not cout
if (fabs(simTk_eta) < etaCut_ && simTk_pt > ptCut_ ) fillHistogram(SimulatedTrackPhi, SimulatedTrackPhiP, SimulatedTrackPhiS, simTk_phi, simTk_type); | ||
|
||
// initialize | ||
for (std::map<unsigned int, DigiMEs>::iterator it = layerMEs.begin(); it != layerMEs.end(); it++) { |
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.
Use range-based loop: auto& it : layerMEs
|
||
int nHitCutoff = 2; | ||
if (pixelFlag_) nHitCutoff = 1; | ||
for (std::map<unsigned int, DigiMEs>::iterator it = layerMEs.begin(); it != layerMEs.end(); it++) { |
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.
use range-based loop
unsigned int rawid = (*isim).detUnitId(); | ||
int layer; | ||
if (pixelFlag_) layer = tTopo->getITPixelLayerNumber(rawid); | ||
else layer = tTopo->getOTLayerNumber(rawid); | ||
if (layer < 0) continue; | ||
|
||
// std::cout << " Layer " << layer << " SimTrack Id " << id << " Type " << simTrk.type() << " Vertex Id " << simTrk.vertIndex() << " Process Type " << (*isim).processType() << " Pt " << simTrk.momentum().pt() << std::endl; |
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.
Delete commented-out code
else local_mes.MissedDigiTrackPt->Fill(pt); | ||
} | ||
if (pt > ptCut_) { | ||
// if (!pixelFlag_ && (std::fabs(eta) > 2.6 || std::fabs(eta) < 0.01) ) std::cout << " Eta " << eta << " Pt " << pt << " layer " << layer << " id " << (*isim).trackId() << " Eloss " << ((*isim).energyLoss()) << " rawid " << rawid << " Event " << iEvent.id().event() << std::endl; |
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.
Delete commented-out code
else local_mes.MatchedSimHitElossP->Fill((*isim).energyLoss()/GeVperElectron); | ||
} else { | ||
local_mes.MissedDigiLocalXposVsYPos->Fill((*isim).localPosition().x(), (*isim).localPosition().y()); | ||
local_mes.MissedDigiTimeWindow->Fill(std::fabs((*isim).timeOfFlight() - pdPos.mag()/30.)); |
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.
magic number 30
// -- Fill NHit per Layer Histogram | ||
// | ||
void Phase2TrackerValidateDigi::fillHitsPerTrack(){ | ||
for (std::map<unsigned int, DigiMEs>::iterator it = layerMEs.begin(); it != layerMEs.end(); it++) { |
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.
use range-based for loop
#include "FWCore/Utilities/interface/InputTag.h" | ||
|
||
#include "DQMServices/Core/interface/DQMEDAnalyzer.h" |
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'm not sure if a dependence on DQM in SIM/DIGI code is allowed
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.
This is allowed in Validation packages. This code will eventually go there before it gets included in official workflow
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.
"Eventually" is rather vague... if this is validation code, it should go in a validation package.
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.
yes, comes up each PR for this package... Lets get these moved to a proper place..
types.push_back("PSS_Modules"); | ||
ibooker.cd(); | ||
|
||
for (std::vector<std::string>::iterator itype = types.begin(); itype != types.end(); itype++) { |
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.
use range-based for loop
please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
Pull request #18687 was updated. @cmsbuild, @civanch, @mdhildreth, @kpedro88, @davidlange6 can you please check and sign again. |
// -- Fill NHit per Layer Histogram | ||
// | ||
void Phase2TrackerValidateDigi::fillHitsPerTrack(){ | ||
for (auto it = layerMEs.begin(); it != layerMEs.end(); it++) { |
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.
use range-based for loop, i.e. for (const auto& it : layerMEs)
@@ -63,12 +64,16 @@ Phase2TrackerValidateDigi::Phase2TrackerValidateDigi(const edm::ParameterSet& iC | |||
itPixelDigiSimLinkToken_(consumes< edm::DetSetVector<PixelDigiSimLink> >(itPixelDigiSimLinkSrc_)), | |||
simTrackToken_(consumes< edm::SimTrackContainer >(simTrackSrc_)), | |||
simVertexToken_(consumes< edm::SimVertexContainer >(simVertexSrc_)), | |||
GeVperElectron(3.61E-09) // 1 electron(3.61eV, 1keV(277e, mod 9/06 d.k. | |||
GeVperElectron(3.61E-09), // 1 electron(3.61eV, 1keV(277e, mod 9/06 d.k. | |||
cval(30.) | |||
{ | |||
for(auto itag : pSimHitSrc_) simHitTokens_.push_back(consumes< edm::PSimHitContainer >(itag)); |
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.
While we're here, let's make this const auto&
types.push_back("PSS_Modules"); | ||
ibooker.cd(); | ||
|
||
for (auto itype : types) { |
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.
use const auto&
Pull request #18687 was updated. @cmsbuild, @civanch, @mdhildreth, @kpedro88, @davidlange6 can you please check and sign again. |
please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
+1 |
+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 |
+1 |
Update of Phase2 Tracker Digitizer validation code :
@delaere @boudoul @atricomi