-
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
new HLT objects monitor code #18879
new HLT objects monitor code #18879
Conversation
A new Pull Request was created by @mtosi (mia tosi) for master. It involves the following packages: DQM/HLTEvF @cmsbuild, @silviodonato, @dmitrijus, @Martin-Grunewald, @fwyzard, @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. |
Pull request #18879 was updated. @cmsbuild, @silviodonato, @dmitrijus, @Martin-Grunewald, @fwyzard, @vanbesien, @davidlange6 can you please check and sign again. |
please abort |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
yes
your 2 suggestions are what i had in mind since last autumn...
… On 7 Jun 2017, at 18:13, Andrea Bocci ***@***.***> wrote:
IIUC, the plots cannot distinguish between
a missing path
a path present in the menu accepting 0 events
I agree that long term we need a more flexible solution. Two possibilities are
adding a "tag" in the Path's description in the HLT menu itself (easy to maintain, but technically complicated to implement)
moving the list of paths to an AlCa-like tag (easy to implement but more complicated to maintain and keep in sync with the menu)
.A
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Hmm, so is there anything else still to be done for this PR? |
please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
ok, @mtosi - i think the only outstanding issue is all the copy and paste of PSets. whats the timescale on which you can have that cleaned up? |
i'm sorry for this, but could we go w/ this PR as it is now
and then i'll make a new PR w/ this fix ?
… On 8 Jun 2017, at 15:17, David Lange ***@***.***> wrote:
ok, @mtosi - i think the only outstanding issue is all the copy and paste of PSets. whats the timescale on which you can have that cleaned up?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
is what i was thinking - but lets agree on a timescale.. |
next week ?
… On 8 Jun 2017, at 15:38, David Lange ***@***.***> wrote:
is what i was thinking - but lets agree on a timescale..
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
+1
… On Jun 8, 2017, at 4:43 PM, mia tosi ***@***.***> wrote:
next week ?
> On 8 Jun 2017, at 15:38, David Lange ***@***.***> wrote:
>
> is what i was thinking - but lets agree on a timescale..
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
great
thanks!
… On Jun 8, 2017, at 4:43 PM, mia tosi ***@***.***> wrote:
next week ?
> On 8 Jun 2017, at 15:38, David Lange ***@***.***> wrote:
>
> is what i was thinking - but lets agree on a timescale..
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
thanks !
On Thu, Jun 8, 2017 at 4:48 PM, David Lange <notifications@github.com>
wrote:
… +1
> On Jun 8, 2017, at 4:43 PM, mia tosi ***@***.***> wrote:
>
> next week ?
>
> > On 8 Jun 2017, at 15:38, David Lange ***@***.***>
wrote:
> >
> > is what i was thinking - but lets agree on a timescale..
> >
> > —
> > You are receiving this because you were mentioned.
> > Reply to this email directly, view it on GitHub, or mute the thread.
> >
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub, or mute the thread.
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#18879 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AEt585FF6ZXybCDCatt-zLJiSqsoXeYQks5sCAmygaJpZM4NiNhd>
.
|
Testing the HLT menu in CMSSW_9_2_2 I found an error due to this PR.
logTest2 contains the following error: while logTest1 is fine. This error was not catch by jenkins test because we have a fake GRun menu in the current release. RelVal_DigiL1RawHLT_GRun_MC.root was generated by |
I think I found the issue ... sorry !!!!!!!!! |
this new code should replace the older version
https://github.com/mtosi/cmssw/blob/c11bf30f20eea09807dc5c69b1ba87b0fd56ae1d/DQM/HLTEvF/plugins/HLTObjectMonitor.cc
I would suggest to leave the old code running at the begging (~few weeks),
because of back compatibility which allows to compare w/ last year plots in case of need
NB: the list of paths and corresponding module names have to be updated as soon as we have the new HLT menu avaialble