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

[90X] Backport HLT (EGamma/MuL3/TkMu) and validation changes for phaseII #20725

Merged
merged 7 commits into from Oct 17, 2017

Conversation

battibass
Copy link

This PR (aimed at superseeding PR 20235):

backports logical changes from PR 20723:

  • makes TSGForOI work in phaseII
  • fixes a small initilaization issue in MaskedMeasurementTrackerEventProducer

includes Egamma changes from PR 20235

backports a change to Phase2OTtiltedBarrelLayer.cc from PR 18801

backports phaseII related changes to TrackerHitAssociator and corresponding cfgs

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 3, 2017

A new Pull Request was created by @battibass (Carlo Battilana) for CMSSW_9_0_X.

It involves the following packages:

DataFormats/EgammaReco
RecoEgamma/EgammaElectronAlgos
RecoEgamma/EgammaElectronProducers
RecoEgamma/EgammaHLTProducers
RecoMuon/TrackerSeedGenerator
RecoTracker/MeasurementDet
RecoTracker/TkDetLayers
SimMuon/MCTruth
SimTracker/TrackerHitAssociation

@perrotta, @cmsbuild, @civanch, @silviodonato, @fwyzard, @mdhildreth, @Martin-Grunewald, @slava77 can you please review it and eventually sign? Thanks.
@ghellwig, @felicepantaleo, @jainshilpi, @abbiendi, @echapon, @threus, @varuns23, @makortel, @sdevissc, @jhgoh, @lgray, @HuguesBrun, @trocino, @rociovilar, @Sam-Harper, @GiacomoSguazzoni, @rovere, @VinInn, @bellan, @wmtford, @mschrode, @ebrondol, @dgulhan, @calderona, @gpetruc, @bachtis this is something you requested to watch as well.
@davidlange6, @slava77 you are the release manager for this.

cms-bot commands are listed here

@slava77
Copy link
Contributor

slava77 commented Oct 3, 2017

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 3, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/23396/console Started: 2017/10/03 15:13

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 3, 2017

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 3, 2017

Comparison job queued.

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 3, 2017

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 110 differences found in the comparisons
  • DQMHistoTests: Total files compared: 23
  • DQMHistoTests: Total histograms compared: 1880671
  • DQMHistoTests: Total failures: 33143
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1847357
  • DQMHistoTests: Total skipped: 171
  • DQMHistoTests: Total Missing objects: 0
  • Checked 94 log files, 9 edm output root files, 23 DQM output files

@Martin-Grunewald
Copy link
Contributor

+1

@perrotta
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 12, 2017

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/23711/console Started: 2017/10/12 10:45

@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-20725/23711/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 110 differences found in the comparisons
  • DQMHistoTests: Total files compared: 23
  • DQMHistoTests: Total histograms compared: 1880705
  • DQMHistoTests: Total failures: 47867
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 1832667
  • DQMHistoTests: Total skipped: 171
  • DQMHistoTests: Total Missing objects: 0
  • Checked 94 log files, 9 edm output root files, 23 DQM output files

@slava77
Copy link
Contributor

slava77 commented Oct 12, 2017

some more bugfixes from #20393 (in 92X since 9_2_12) are not included.
Is this intentional?
@battibass please clarify.
Thank you.

@battibass
Copy link
Author

@slava77 : it was not intended but, discussing with @folguera, we agreed that the impact of the bugfix (~1% eff) is not worth to be included (also considering that the L3 has evolved after 90X beyond TSForOI, and the other changes are not backported).
Unless you or @gennai would consider the backport necessary, I would skip it.

@slava77
Copy link
Contributor

slava77 commented Oct 13, 2017

+1

for #20725 c67a50a

  • backports related of Egamma and muon (mostly) HLT-related developments from 92X to 90X.
  • jenkins tests pass and comparisons with baseline show roughly expected behavior:
    • non-phase-2 workflows have differences in electronMergedSeeds due to change in format [useful data does not change: there are no changes downstream]
    • phase-2 workflows also have small changes in muon and track reco related to fixes in MaskedMeasurementTrackerEventProducer.cc and Phase2OTtiltedBarrelLayer.cc.

Since fixes from #20393 appear to be not essential and that they appeared after the earlier version of this PR #20235), I'm assuming it's good to go now.

@Martin-Grunewald
Copy link
Contributor

+1

@civanch
Copy link
Contributor

civanch commented Oct 15, 2017

+1

@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) and once validation in the development release cycle CMSSW_9_4_X is complete. 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)

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 5b14024 into cms-sw:CMSSW_9_0_X Oct 17, 2017
@battibass battibass deleted the muonHLTPhase2_90X branch June 8, 2023 12:06
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

8 participants