Skip to content
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

Muon HLT - fix in TSGForOI #17750

Merged
merged 3 commits into from Mar 20, 2017

Conversation

folguera
Copy link
Contributor

@folguera folguera commented Mar 2, 2017

Rescale the TSOS error before finding the compatible detector.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 2, 2017

A new Pull Request was created by @folguera (Santiago Folgueras) for CMSSW_9_0_X.

It involves the following packages:

RecoMuon/TrackerSeedGenerator

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@bellan, @abbiendi, @jhgoh, @echapon, @calderona, @HuguesBrun, @battibass, @trocino, @bachtis, @rociovilar this is something you requested to watch as well.
@davidlange6, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

@folguera folguera changed the title Muon hlt iter l3 tsg for o ifix Muon HLT - fix in TSGForOI Mar 2, 2017
@slava77
Copy link
Contributor

slava77 commented Mar 3, 2017

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 3, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18137/console Started: 2017/03/03 21:57

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 3, 2017

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 3, 2017

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 3, 2017

@perrotta
Copy link
Contributor

perrotta commented Mar 9, 2017

@folguera : the fix is quite reasonable. I don't see any visible effect in the jenkins outputs, though: I imagine that some larger samples are needed in order to be able to see any reflection on the validation plots. Do you have any suggestion about how to quickly spot the effect of this fix? Could you please point out a presentation in which you showed it already?
In any case, I think this should go into 91X first: please provide a PR also for that cycle

@folguera
Copy link
Contributor Author

folguera commented Mar 9, 2017

@perrotta I cannot think of a quick way of spotting the difference, we simply though it made more sense to rescale before finding the compatible hits rather than doing it later... in the validation plots we ran offline we did not see any effect in terms of performance, I can try to find that plot for you.

In any case we will provide a PR in 91X.

@folguera
Copy link
Contributor Author

folguera commented Mar 9, 2017

@perrotta I've created the PR in 91X (#17862) and the branch was immediately closed arguing that this branch is closed for updates.

@davidlange6
Copy link
Contributor

davidlange6 commented Mar 9, 2017 via email

@perrotta
Copy link
Contributor

+1
Same as #17917 in 91X
No changes visible in jenkin tests, but the effect of the fix on a larger stat sample shown in #17917 (comment) goes in the wanted direction

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_9_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit eb9c24b into cms-sw:CMSSW_9_0_X Mar 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants