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
Track cuts redone #19428
Track cuts redone #19428
Conversation
A new Pull Request was created by @fioriNTU for master. It involves the following packages: DQM/SiPixelPhase1Clusters @vazzolini, @kmaeshima, @dmitrijus, @cmsbuild, @vanbesien, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
The tests are being triggered in jenkins. |
@@ -142,6 +161,7 @@ void SiPixelPhase1TrackClusters::analyze(const edm::Event& iEvent, const edm::Ev | |||
histo[ONTRACK_SIZE ].fill(double(cluster.size() ), id, &iEvent); | |||
histo[ONTRACK_POSITION_B].fill(clustgp.z(), clustgp.phi(), id, &iEvent); | |||
histo[ONTRACK_POSITION_F].fill(clustgp.x(), clustgp.y(), id, &iEvent); | |||
histo[ONTRACK_SIZE_VS_ETA].fill(etatk[key], cluster.sizeY(), id, &iEvent); |
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.
strickly speaking one should take into account bigpixel for instance as
auto sizeY = cluster.sizeY();
if (topol.containsBigPixelInY(cluster.minPixelCol(), cluster.maxPixelCol()) ) sizeY+=1;
we need to ask confirmation to Pixels guys....
} | ||
|
||
void SiPixelPhase1TrackClusters::analyze(const edm::Event& iEvent, const edm::EventSetup& iSetup) { | ||
|
||
if (! (histo.size() > ONTRACK_SIZE_VS_ETA)) return; |
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 a QUICK fix.
we may have to issue a LogWarning in future (actually an assert)
-1 Tested at: 94ba37c The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: You can see the results of the tests here: I found follow errors while testing this PR Failed tests: AddOn
I found errors in the following addon tests: cmsDriver.py RelVal -s L1REPACK:Full2015Data --data --scenario=HeavyIons -n 10 --conditions auto:run2_hlt_HIon --relval 9000,50 --datatier "RAW" --eventcontent RAW --customise=HLTrigger/Configuration/CustomConfigs.L1T --era Run2_2016,Run2_HI --magField 38T_PostLS1 --fileout file:RelVal_Raw_HIon_DATA.root --filein /store/hidata/HIRun2015/HIHardProbes/RAW-RECO/HighPtJet-PromptReco-v1/000/263/689/00000/1802CD9A-DDB8-E511-9CF9-02163E0138CA.root : FAILED - time: date Mon Jun 26 19:25:42 2017-date Mon Jun 26 19:17:00 2017 s - exit: 23552 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
Comparison job queued. |
@dmitrijus @davidlange6 what shall I do to fix this? A rebase? |
Comparison is ready Comparison Summary:
|
@dmitrijus and @davidlange6 sorry to insist, but I need help in fixing this. What I did is to cherry-pick my commits from the previous PR and add the Vincenzo's suggestions on top of it. |
I think the failures are not related |
please test |
+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:
|
@dmitrijus , could you please consider this PR ? thanks |
3 similar comments
@dmitrijus , could you please consider this PR ? thanks |
@dmitrijus , could you please consider this PR ? thanks |
@dmitrijus , could you please consider this PR ? thanks |
I would like to add stuff to this, possibly in time for next round of data taking... |
The integration of this PR is becoming extremely urgent |
+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 |
@@ -41,8 +42,16 @@ void SiPixelPhase1RecHits::analyze(const edm::Event& iEvent, const edm::EventSet | |||
iEvent.getByToken( srcToken_, tracks); | |||
if (!tracks.isValid()) return; | |||
|
|||
edm::Handle<reco::VertexCollection> vertices; |
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 @fioriNTU - please do the getByToken (and the consumes above) only if applyVertexCut_ is true
Also replace vertices->size()==0 by vertices->empty()
@davidlange6 , please integrate |
+1 |
@fioriNTU could you please back port this PR to 92X? It is needed for the online DQM, otherwise your changes don't get merged into the current production branch. |
This PR replicates the #19194 that has been merged and then reverted before 9_2_4
A protection against crashes has been added following indications of @davidlange6 and @VinInn