Dt segment fit update7 x

[namapane]

Moving to 71X, as requested in:

#1690 (comment)

[cmsbuild]

A new Pull Request was created by @namapane (Nicola Amapane) for CMSSW_7_1_X.

Dt segment fit update7 x

It involves the following packages:

RecoLocalMuon/DTSegment
RecoMuon/MuonIdentification
RecoMuon/TrackingTools
Validation/DTRecHits

@thspeer, @danduggan, @rovere, @cmsbuild, @anton-a, @nclopezo, @deguio, @slava77, @Degano, @ojeda can you please review it and eventually sign? Thanks.
@bachtis this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
@ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

[cmsbuild]

+1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2133/32/summary.html

[slava77]

I think there is enough time to fix if conveners or HLT contacts look at the configuration changes.

Conveners seem to be OK with the regression in HLT, so I will approve for pre4 around the deadline, but leaving some time to fix it now.

[slava77]

Here are a few more plots, this time for single mu pt 1000 (matrix workflow 22.0), looking at just hlt L2 and L3 muons (red is new, with the changes in DT). The effect seems larger for high-pt muons in this case.

[abbiendi]

Hi Slava,

we have also done some private validation with 2K single muon pt=10 GeV, looking at the HLT efficiency vs Gen level for the full HLT single muon path and for L2 Vs L1. There is almost no visible change (see the plots). The message is that we don't loose any trigger efficiency. The few hits that are lost in L2Muons could hint to a possible optimization of the MT algo in HLT, as it is quite sharp in collecting the hits (delta-rays are automatically rejected). The situation could be patched at any time by the developers introducing a parameter that would inactivate the MT in HLT, but as I said, now they are unavailable in time for pre4. Anyway to me it seems that this is really a minor aspect in comparison with the major advances that this PR introduces. And, as I said, we will follow up in detail with our standard validation. I thinks this is the normal correct procedure. From the Muon POG point of view we give green-light to put this into 71X for pre4. The DT DPG clearly already agrees as they submitted the PR.
cheers
Giovanni

[slava77]

+1

for #2133 09327b3
tested in CMSSW_7_1_X_2014-02-24-1400/sign311
decision based on the review posted in this PR

[namapane]

Dear all,

Apologies for the slow feedback, I am going through my emails now. A slight reduction of the number of hits is expected because MT is more efficient in rejecting deltas, although I thought that Piotr had set it up to minimize the differences on HLT (to be conservative). Piotr is the real expert here so he will be able to tell all the details, but in the meanwhile, I fully agree with Giovanni.

Cheers

Nicola

---

From: abbiendi [notifications@github.com]
Sent: 28 February 2014 20:33
To: cms-sw/cmssw
Cc: Nicola Amapane
Subject: Re: [cmssw] Dt segment fit update7 x (#2133)

Hi Slava,

we have also done some private validation with 2K single muon pt=10 GeV, looking at the HLT efficiency vs Gen level for the full HLT single muon path and for L2 Vs L1. There is almost no visible change (see the plots). The message is that we don't loose any trigger efficiency. The few hits that are lost in L2Muons could hint to a possible optimization of the MT algo in HLT, as it is quite sharp in collecting the hits (delta-rays are automatically rejected). The situation could be patched at any time by the developers introducing a parameter that would inactivate the MT in HLT, but as I said, now they are unavailable in time for pre4. Anyway to me it seems that this is really a minor aspect in comparison with the major advances that this PR introduces. And, as I said, we will follow up in detail with our standard validation. I thinks this is the normal correct procedure. From the Muon POG point of view we give green-light to put this into 71X for pre4. The DT DPG clearly al ready ag rees as they submitted the PR.
cheers
Giovanni

[mt-71x_l2wrtl1_efficiencyvseta]https://f.cloud.github.com/assets/5655983/2297598/1492bee2-a0af-11e3-82f4-f3a8cc66d0c1.png

[mt-71x_l3efficiencyvseta]https://f.cloud.github.com/assets/5655983/2297602/25d50df4-a0af-11e3-8ac5-4e9df78f2872.png

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/2133#issuecomment-36386110.

[deguio]

ciao @namapane
do you mind migrate the modules to use the consumes interface please? I still see some getByLabel here and there. we are also migrating the analysers to be DQMEDAnalyzers as described in https://twiki.cern.ch/twiki/bin/viewauth/CMS/ThreadedDQM

thanks for your help,
F.

[ptraczyk]

hi
thanks for following up on this while I was away last week. if this issue
with hits being lost is only happening in L2 then I suspect this might be
due to the delta ray rejection not being switched off in the configuration.
It is no longer necessary since Meantimer takes care of deltas internally.
Maybe this slipped through accidentally. I'm taking a look now.

best,
Piotr

On Mon, Mar 3, 2014 at 5:02 PM, deguio notifications@github.com wrote:

ciao @namapane https://github.com/namapane
do you mind migrate the modules to use the consumes interface please? I
still see some getByLabel here and there. we are also migrating the
analysers to be DQMEDAnalyzers as described in
https://twiki.cern.ch/twiki/bin/viewauth/CMS/ThreadedDQM

thanks for your help,
F.

Reply to this email directly or view it on GitHubhttps://github.com//pull/2133#issuecomment-36524498
.

[ptraczyk]

update: I didn't seem to remember changing any HLT configs and indeed
unless I'm reading something wrong, HLT is still using the old
Combinatorial Pattern RECO since we never really studied using Meantimer at
HLT. Since some of the code that I was changing is used by both pattern
reco algorithms, this is probably responsible for the change in hit numbers.
At this point I agree that we should proceed with the changes since L2 muon
performance is not being degraded - and it would be good to look into the
possibility of enabling Meantimer in HLT also, to re-align HLT and Offline.

On Tue, Mar 4, 2014 at 1:29 PM, Piotr Traczyk piotraczyk@gmail.com wrote:

hi
thanks for following up on this while I was away last week. if this issue
with hits being lost is only happening in L2 then I suspect this might be
due to the delta ray rejection not being switched off in the configuration.
It is no longer necessary since Meantimer takes care of deltas internally.
Maybe this slipped through accidentally. I'm taking a look now.

best,
Piotr

On Mon, Mar 3, 2014 at 5:02 PM, deguio notifications@github.com wrote:

ciao @namapane https://github.com/namapane
do you mind migrate the modules to use the consumes interface please? I
still see some getByLabel here and there. we are also migrating the
analysers to be DQMEDAnalyzers as described in
https://twiki.cern.ch/twiki/bin/viewauth/CMS/ThreadedDQM

thanks for your help,
F.

Reply to this email directly or view it on GitHubhttps://github.com//pull/2133#issuecomment-36524498
.

[abbiendi]

From the Muon POG point of view I agree. And even if the HLT performance seems unchanged this pre-release will not be used for any HLT studies.

[perrotta]

Ciao Giovanni @abbiendi.

@Martin-Grunewald

Indeed, the second part of your statement is wrong (HLT development is
going on in 71X, and therefore the development studies must be
performed over there as well).

I agree that the new meantimer be made available also for HLT: at the
end, our aim is to run the same modules online and offline, whenever
possible. This needs to be studied, however, and probably some fine
tuning of the parameters (e.g. delta rejection) will also be needed.

In the meanwhile: Piotr @ptraczyk, could you please make configurable
the changes in the part of the code which potentially affects HLT, so
that the old HLT behaviour can be preserved? Ideally, the default
parameters should allow the old behaviour, while some modified one
will allow us to switch to the new meantimer when needed/wanted.

Thank you,
Andrea

abbiendi notifications@github.com ha scritto:

From the Muon POG point of view I agree. And even if the HLT
performance seems unchanged this pre-release will not be used for
any HLT studies.

---

Reply to this email directly or view it on GitHub:
#2133 (comment)

---

This message was sent using IMP, the Internet Messaging Program.

[davidlange6]

@deguio , @danduggan - is this PR ok for you?

[danduggan]

@davidlange6, @deguio is following this PR and can confirm - he's offline at the moment though. is this an urgent request? if so i can pick it up.

[davidlange6]

just trying to dislodge old pull requests pending in 71x - not urgent from my side

On Mar 6, 2014, at 11:39 AM, danduggan notifications@github.com
wrote:

@davidlange6, @deguio is following this PR and can confirm - he's offline at the moment though. is this an urgent request? if so i can pick it up.

—
Reply to this email directly or view it on GitHub.

[deguio]

hello @davidlange6 all,
this is in principle fine with DQM, but, as I commented before, I would prefer having all the getByLabel migrated to the new consumes syntax before merging.

all the modules in Validation/DTRecHits still have to be migrated. should we factorise and ask for this changes in a separate pull request?

thanks,
F.

[davidlange6]

I'm definitely in favor of getting the consumes migration done as part of this PR. I missed that comment - is someone confirmed to be working on it?

On Mar 7, 2014, at 12:18 PM, deguio notifications@github.com
wrote:

hello @davidlange6 all,
this is in principle fine with DQM, but, as I commented before, I would prefer having all the getByLabel migrated to the new consumes syntax before merging.

all the modules in Validation/DTRecHits still have to be migrated. should we factorise and ask for this changes in a separate pull request?

thanks,
F.

—
Reply to this email directly or view it on GitHub.

[namapane]

Hi,

I would have a strong preference for having this branch merged before
the consumes migration. The reason (as we discussed in the past) is that
we are maintaining a private backport of this branch to 5X in order to
be able to study performance with real data, and introducing
backward-incompatible changes in the same files will make this even more
painful. For us it would be much easier to have these developments
finally integrated, and have a separate pull request for consumes on top
of it.

Cheers,
Nicola

On 07-Mar-14 12:26, davidlange6 wrote:

I'm definitely in favor of getting the consumes migration done as part
of this PR. I missed that comment - is someone confirmed to be working
on it?

On Mar 7, 2014, at 12:18 PM, deguio notifications@github.com
wrote:

hello @davidlange6 all,
this is in principle fine with DQM, but, as I commented before, I
would prefer having all the getByLabel migrated to the new consumes
syntax before merging.

all the modules in Validation/DTRecHits still have to be migrated.
should we factorise and ask for this changes in a separate pull request?

thanks,
F.

—
Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHub
#2133 (comment).

[deguio]

+1
I keep in mind that another pull will come containing the changes needed to migrate to the consumes syntax. I am going to migrate the remaining packages then.. :)
thanks,
F.

[cmsbuild]

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?