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
Initial push for LumiZCounting #18899
Conversation
A new Pull Request was created by @xmniu (Xinmei Niu) for master. It involves the following packages: DQMOffline/LumiZCounting The following packages do not have a category, yet: DQMOffline/LumiZCounting @cmsbuild, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
<use name="TrackingTools/IPTools"/> | ||
<use name="boost"/> | ||
<use name="root"/> | ||
<flags CXXFLAGS="-g -Wall"/> |
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 remove this. If you want to use "-g" or other flags locally, please use export USER_CXXFLAGS="-ggdb"
.
But don't put them into module scope!
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.
Will do
<use name="DQMServices/Core"/> | ||
<use name="DataFormats/CLHEP"/> | ||
<use name="DQMOffline/LumiZCounting"/> | ||
<flags CXXFLAGS="-g -Wall"/> |
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.
Same 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.
will do
@@ -0,0 +1,98 @@ | |||
// SingleMu |
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.
Why not put these files into python configuration file?
I think it would cleaner that way (and less ways in which things can break: no file discovery, etc.)
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.
The files in data/ and the data/ subfolder will be removed. Actually the trigger menu is not needed, since we plan to use one same trigger over entire data taking period.
typedef std::pair<unsigned int, unsigned int> RunLumiPairType; | ||
typedef std::map<unsigned int, std::vector<RunLumiPairType>> MapType; | ||
|
||
RunLumiRangeMap(){} |
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.
Is this code used? If not, please remove it or move to "test/" subdirectory (if for some reason you want/need to preserve 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.
Not for now, will remove to test/ since it might be used if we decide to do a lumi mask in the future.
// | ||
void ZCounting::setTriggers() | ||
{ | ||
std::string cmssw_base_src = getenv("CMSSW_BASE"); cmssw_base_src+="/src/"; |
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 will most likely fail in any production environment. That is why it would be for the best to put those files into python configuration (PSet() and friends).
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 will be removed together with data/.
edm::EDGetTokenT<reco::TrackCollection> fTrackName_token; | ||
|
||
// bacon fillers | ||
baconhep::TTrigger *fTrigger; |
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 use std::unique_ptr<baconhep::TTrigger>
.
please test |
The tests are being triggered in jenkins. |
I have added few comments / questions. The most important issue is the use of "data/" files, I am sure they are broken :) |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
112354c
to
30822b0
Compare
Pull request #18899 was updated. @cmsbuild, @davidlange6 can you please check and sign again. |
@dmitrijus your comments have been implemented, please check again. thanks. |
DQMOffline/Lumi/plugins/ZCounting.cc
Outdated
{//Muon isolation selection, up-to-date with MUO POG recommendation | ||
|
||
if(isoType == TrackerIso && muon.isolationR03().sumPt < isoCut) return true; | ||
else if(isoType == PFIso && muon.pfIsolationR04().sumChargedHadronPt + TMath::Max(0.,muon.pfIsolationR04().sumNeutralHadronEt + muon.pfIsolationR04().sumPhotonEt - 0.5*muon.pfIsolationR04().sumPUPt) < isoCut) return true; |
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.
hi @xmniu - sorry missed this - TMath::Max can just be std::max (then you can remove all the TMath.h includes
DQMOffline/Lumi/plugins/ZCounting.cc
Outdated
|
||
// Mass window | ||
TLorentzVector vDilep = vTag + vProbe; | ||
if((vDilep.M() < MassMin_) || (vDilep.M() > MassMax_)) continue; |
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 make a variable for vDilep.M() to avoid redundant calculations- you use it many times.
class TriggerRecord | ||
{ | ||
public: | ||
TriggerRecord(const std::string name="", const unsigned int value=0) { |
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.
one more const std::string &
otherwise looks ready to me |
…ng to new HLT menu of 2017B
7193475
to
f79b783
Compare
Pull request #18899 was updated. @vazzolini, @dmitrijus, @kmaeshima, @kpedro88, @fabozzi, @cmsbuild, @kkousour, @GurpreetSinghChahal, @vanbesien, @davidlange6 can you please check and sign again. |
please test |
The tests are being triggered in jenkins. |
+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:
|
merge |
This is the initial push for DQM LumiZCounting.
14 2D histograms are booked in order to calculate Z yields and efficiencies, for a certain time window, say a couple of lumi-sections.