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
BPH HLT DQM #18224
BPH HLT DQM #18224
Conversation
A new Pull Request was created by @anigamova for master. It involves the following packages: DQMOffline/Trigger @cmsbuild, @dmitrijus, @vanbesien, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here #13028 |
METME mud0_; | ||
METME muz0_; | ||
|
||
METME JpsiPhi_; |
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 you do not use these MEs, please drop
GenericTriggerEventFlag* num_genTriggerEventFlag_; | ||
GenericTriggerEventFlag* den_genTriggerEventFlag_; | ||
|
||
// StringCutObjectSelector<reco::MET,true> metSelection_; |
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 this lines, please
xmin = cms.double( -0.5), | ||
xmax = cms.double(200), | ||
) | ||
hltBPHmonitoring.histoPSet.phiPSet = cms.PSet( |
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.
do you really need to specify the phi in the configuration ?
shouldn't it be fixed ?
xmin = cms.double( -3.2), | ||
xmax = cms.double(3.2), | ||
) | ||
hltBPHmonitoring.histoPSet.etaPSet = cms.PSet( |
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 binning enough ?
shouldn't you have a binning more related to the detector features as well ?
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 think 0.2 will be enough
xmin = cms.double( -2.6), | ||
xmax = cms.double(2.6), | ||
) | ||
hltBPHmonitoring.histoPSet.d0PSet = cms.PSet( |
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.
1 cm ?
I think it would better to increase the resolution, why don't ou make use of a variable binning here ?
|
||
edm::EDGetTokenT<reco::MuonCollection> muoToken_; | ||
edm::EDGetTokenT<reco::BeamSpot> bsToken_; | ||
edm::EDGetTokenT<reco::VertexCollection> PVsToken_; |
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.
are you using the PV ?
if not, please drop
@@ -18,6 +18,7 @@ | |||
from DQMOffline.Trigger.TrackingMonitoringPA_Client_cff import * | |||
|
|||
from DQMOffline.Trigger.ExoticaMonitoring_Client_cff import * | |||
<<<<<<< HEAD |
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 think there is something wrong here ...
please, clean this code
can we test it ? |
@dmitrijus @vanbesien , could we give a try, please ? |
please test |
The tests are being triggered in jenkins. |
Pull request #18224 was updated. @cmsbuild, @dmitrijus, @vanbesien, @davidlange6 can you please check and sign again. |
Pull request #18224 was updated. @cmsbuild, @dmitrijus, @vanbesien, @davidlange6 can you please check and sign again. |
Pull request #18224 was updated. @cmsbuild, @dmitrijus, @vanbesien, @davidlange6 can you please check and sign again. |
@davidlange6 thanks for comments, fixed everything |
@davidlange6 do we have to test it again ? |
yes, starting now |
please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
genericTriggerEventPSet.add<std::vector<std::string> >("hltPaths",{}); | ||
genericTriggerEventPSet.add<std::string>("hltDBKey",""); | ||
genericTriggerEventPSet.add<bool>("errorReplyHlt",false); | ||
genericTriggerEventPSet.add<unsigned int>("verbosityLevel",1); |
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 @anigamova - a late followup on this pull request - for production running this parameter needs to be 0. Please make a new PR with that change. 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.
Hi, created PR #18694
Thanks
reference method