fix # MEs in pixel DQM@HLT as requested by #20192 #20298
Conversation
|
The code-checks are being triggered in jenkins. |
|
A new Pull Request was created by @mtosi (mia tosi) for master. It involves the following packages: DQMOffline/Trigger @kmaeshima, @cmsbuild, @vanbesien, @vazzolini, @dmitrijus can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
+code-checks |
| ### IT HAS TO BE IN SYNCH W/ | ||
| ### THE LIST DEFINED IN THE ENUM | ||
| ### https://cmssdt.cern.ch/lxr/source/DQM/SiPixelPhase1TrackClusters/src/SiPixelPhase1TrackClusters.cc#0063 | ||
| SiPixelPhase1TrackClustersOnTrackCharge, |
There was a problem hiding this comment.
this and all others are now "pointing" to the offline object not to the hlt one...
QUESTION:
WHY there is a need to duplicate the whole offline code?
can you not just use the offline definitions?
There was a problem hiding this comment.
indeed, I was too fast :(
I redefined the list becasue we generally want the summary ones only
|
The code-checks are being triggered in jenkins. |
|
Pull request #20298 was updated. @kmaeshima, @cmsbuild, @vanbesien, @vazzolini, @dmitrijus can you please check and sign again. |
|
please test |
|
The tests are being triggered in jenkins. |
|
+code-checks |
|
-1 Tested at: 5f564c8 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: RelVals
When I ran the RelVals I found an error in the following worklfows: runTheMatrix-results/136.788_RunSinglePh2017B+RunSinglePh2017B+HLTDR2_2017+RECODR2_2017reHLT_skimSinglePh_Prompt+HARVEST2017/step3_RunSinglePh2017B+RunSinglePh2017B+HLTDR2_2017+RECODR2_2017reHLT_skimSinglePh_Prompt+HARVEST2017.log10042.0 step3 runTheMatrix-results/10042.0_ZMM_13+ZMM_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+ALCAFull_2017+HARVESTFull_2017/step3_ZMM_13+ZMM_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+ALCAFull_2017+HARVESTFull_2017.log10024.0 step3 runTheMatrix-results/10024.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+ALCAFull_2017+HARVESTFull_2017/step3_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+ALCAFull_2017+HARVESTFull_2017.log10824.0 step3 runTheMatrix-results/10824.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2018_GenSimFull+DigiFull_2018+RecoFull_2018+ALCAFull_2018+HARVESTFull_2018/step3_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2018_GenSimFull+DigiFull_2018+RecoFull_2018+ALCAFull_2018+HARVESTFull_2018.log11624.0 step3 runTheMatrix-results/11624.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2019_GenSimFull+DigiFull_2019+RecoFull_2019+ALCAFull_2019+HARVESTFull_2019/step3_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2019_GenSimFull+DigiFull_2019+RecoFull_2019+ALCAFull_2019+HARVESTFull_2019.log The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
|
Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped) |
|
by explicitly listing all the PSets as they are defined in the enum I added the check of the validity of the PixelClusterShapeCache handle in the source code as well |
|
@cmsbuild, 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:
|
|
+1 |
cmsbuild
added
dqm-approved
fully-signed
and removed
dqm-pending
pending-signatures
labels
|
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 will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar (and backports should be raised in the release meeting by the corresponding L2) |
|
|
||
| hltSiPixelPhase1TrackClustersNTracks, | ||
| hltSiPixelPhase1TrackClustersNTracksInVolume, | ||
| ### IT HAS TO BE IN SYNCH W/ |
There was a problem hiding this comment.
hi @mtosi all - is there some technical reason this magic list can not be moved to python, which can be both shared across python modules and communicated to c++ (the reverse, as done now, sharing information in c++ to python, being hard)
There was a problem hiding this comment.
This is how Marcel design it. the whole pixelPhase1 DQM seems to work in that way.
I do not think it is optimal. I suspect we need to wait for Marcel to be back in CMS and re-design it...
There was a problem hiding this comment.
sigh- ok, hopefully the fragileness is now documented so that things stay in sync..
|
I let pixel DQM answer ..
…On Tue, Sep 5, 2017 at 4:31 PM, David Lange ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In DQMOffline/Trigger/python/SiPixel_OfflineMonitoring_TrackCluster_cff.py
<#20298 (comment)>:
> )
hltSiPixelPhase1TrackClustersConf = cms.VPSet(
- hltSiPixelPhase1TrackClustersOnTrackCharge,
- hltSiPixelPhase1TrackClustersOnTrackSize,
- hltSiPixelPhase1TrackClustersOnTrackNClusters,
- hltSiPixelPhase1TrackClustersOnTrackPositionB,
- hltSiPixelPhase1TrackClustersOnTrackPositionF,
-
- hltSiPixelPhase1TrackClustersOffTrackCharge,
- hltSiPixelPhase1TrackClustersOffTrackSize,
- hltSiPixelPhase1TrackClustersOffTrackNClusters,
- hltSiPixelPhase1TrackClustersOffTrackPositionB,
- hltSiPixelPhase1TrackClustersOffTrackPositionF,
-
- hltSiPixelPhase1TrackClustersNTracks,
- hltSiPixelPhase1TrackClustersNTracksInVolume,
+### IT HAS TO BE IN SYNCH W/
hi @mtosi <https://github.com/mtosi> all - is there some technical reason
this magic list can not be moved to python, which can be both shared across
python modules and communicated to c++ (the reverse, as done now, sharing
information in c++ to python, being hard)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#20298 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEt58547CQDbJ73CbdryZmzyapdXQBvfks5sfVs-gaJpZM4PGBjD>
.
|
|
@davidlange6 actually this is a question more for @VinInn |
|
+1 |
as requested/suggested by #20192
@VinInn @perrotta @fioriNTU
let me know if we need this fix in 92x as well
thanks