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

update L3 filter, for 2017 online muon reconstruction #19465

Merged
merged 4 commits into from Jul 5, 2017

Conversation

sarafiorendi
Copy link
Contributor

Update of the L3 filter in order to:

  • handle muons reconstructed by the full (L3FromL2 + L3FromL1) iterL3 sequence, and perform the matching with the candidate from the previous step
  • introduce the possibility of disabling the matching of the L3 to the previous candidate (L2 or L1)

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @sarafiorendi for master.

It involves the following packages:

HLTrigger/Muon

@Martin-Grunewald, @silviodonato, @cmsbuild, @fwyzard, @davidlange6 can you please review it and eventually sign? Thanks.
@battibass, @abbiendi, @jhgoh, @Martin-Grunewald, @calderona, @HuguesBrun, @trocino this is something you requested to watch as well.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

}
} //MTL loop


if ( !( l1CandTag_ == edm::InputTag("")) && check_l1match && matchPreviousCand_){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you use

if (not l1CandTag_.label().empty() ...

instead, to avoid creating InputTags every time ?

@fwyzard
Copy link
Contributor

fwyzard commented Jun 28, 2017

@cmsbuild , please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jun 28, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/20973/console Started: 2017/06/28 21:20

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19465/20973/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 22
  • DQMHistoTests: Total histograms compared: 1756063
  • DQMHistoTests: Total failures: 1231
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1754666
  • DQMHistoTests: Total skipped: 166
  • DQMHistoTests: Total Missing objects: 0
  • Checked 90 log files, 14 edm output root files, 22 DQM output files

@Martin-Grunewald
Copy link
Contributor

Presumably we need a PR for 92X as well - please make one!

@Martin-Grunewald
Copy link
Contributor

Martin-Grunewald commented Jun 30, 2017

This change kills off a few triggers (on 100 ttbar events) - new vs old:

HLT-Report ---------- Event  Summary ------------
HLT-Report Summary for Job
HLT-Report Events total = 100 wasrun = 100 passed = 100 errors = 0

HLT-Report ---------- HLTrig Summary ------------
HLT-Report   HLT #  WasRun     L1S     Pre     HLT   %L1sPre    Rate  RateHi  Errors Name

***************
*** 6204 ****
! HLT-Report      24     100     100      49       0   0.00000     0.0     0.0       0 HLT_Mu3_PFJet40_v8
--- 6261 ----
! HLT-Report      24     100     100      49      12  24.48980     0.0     0.0       0 HLT_Mu3_PFJet40_v8
***************
*** 6355 ****
! HLT-Report     175     100     100      49       0   0.00000     0.0     0.0       0 HLT_Mu8_TrkIsoVVL_v7
--- 6412 ----
! HLT-Report     175     100     100      49       8  16.32653     0.0     0.0       0 HLT_Mu8_TrkIsoVVL_v7
***************
*** 6362 ****
! HLT-Report     182     100     100      38       0   0.00000     0.0     0.0       0 HLT_Mu17_TrkIsoVVL_v7
--- 6419 ----
! HLT-Report     182     100     100      38       6  15.78947     0.0     0.0       0 HLT_Mu17_TrkIsoVVL_v7
***************
*** 6440,6441 ****
! HLT-Report     260     100     100      49       0   0.00000     0.0     0.0       0 HLT_Mu8_v7
! HLT-Report     261     100     100      38       0   0.00000     0.0     0.0       0 HLT_Mu17_v7
--- 6497,6498 ----
! HLT-Report     260     100     100      49       8  16.32653     0.0     0.0       0 HLT_Mu8_v7
! HLT-Report     261     100     100      38       6  15.78947     0.0     0.0       0 HLT_Mu17_v7
***************

eg, for HLT_Mu8, the event flow through all modules is

TrigReport     1   64         45         45          0          0 hltL3MuonsIterL3IO
TrigReport     1   65         45         45          0          0 hltIterL3MuonsFromL2LinksCombination
TrigReport     1   66         45         45          0          0 hltIterL3MuonsFromL2
TrigReport     1   67         45         45          0          0 hltIterL3FromL2MuonCandidates
TrigReport     1   68         45          0         45          0 hltL3fL1sMu5L1f0L2f5L3Filtered8
TrigReport     1   69          0          0          0          0 hltBoolEnd

where this is the killing filter:

process.hltL3fL1sMu5L1f0L2f5L3Filtered8 = cms.EDFilter( "HLTMuonL3PreFilter",

Please configure the default working of the new features to reproduce the old behaviour!

@sarafiorendi
Copy link
Contributor Author

Hi @Martin-Grunewald ,

this is expected, and in particular it is due to the change here
https://github.com/cms-sw/cmssw/pull/19465/files#diff-6820f725f6447899fb4b62476d21b33cR214.
In short:

  • this filter is currently used only for paths where the "L3FromL2" muon sequence is used, which produces L3 candidates and their link collection
  • to correctly match each L3 candidate with its link, we should compare the globalTrack

The modification in this PR are meant to update the filter to be used after L3 candidates are produced using the full iterL3 sequence.
In this case, due to the way the sequence is built, we need to use the trackerTrack in order to correctly match the L3 candidates with their link.
The usage of the updated filter with the current menu will kill all the paths where the filter is used after the "L3FromL2" sequence.

To conclude, I can introduce another configurable parameter which would make the filter use the globalTrack until we finally switch to the new menu, where L3FromL2 is replaced by full iterL3 everywhere.
Would you agree?

thanks

@fwyzard
Copy link
Contributor

fwyzard commented Jul 1, 2017

If the new parameter is used only during the transition, I would rather not add it.

Instead, can we pick one of these options:

  • add a python customisation that adjusts this module's configuration until the migration, or
  • parse this in a private branch, and update the menu and filters in a single PR

.A

@Martin-Grunewald
Copy link
Contributor

+1
OK, just that trigger proponents are aware - and have the added incentive to migrate!

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 3, 2017

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

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 4, 2017

Pull request #19465 was updated. @Martin-Grunewald, @silviodonato, @cmsbuild, @fwyzard, @davidlange6 can you please check and sign again.

@fwyzard
Copy link
Contributor

fwyzard commented Jul 4, 2017

@cmsbuild, please test

@fwyzard
Copy link
Contributor

fwyzard commented Jul 4, 2017

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 4, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/21157/console Started: 2017/07/04 23:36

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 4, 2017

This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 4, 2017

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 4, 2017

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 4, 2017

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19465/21157/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 22
  • DQMHistoTests: Total histograms compared: 1760883
  • DQMHistoTests: Total failures: 30173
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1730544
  • DQMHistoTests: Total skipped: 166
  • DQMHistoTests: Total Missing objects: 0
  • Checked 90 log files, 14 edm output root files, 22 DQM output files

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 3ae8d8c into cms-sw:master Jul 5, 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