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

Phase2 L1 ParticleFlow #30510

Merged
merged 9 commits into from Jul 3, 2020
Merged

Phase2 L1 ParticleFlow #30510

merged 9 commits into from Jul 3, 2020

Conversation

silviodonato
Copy link
Contributor

Based on #30482.

PR description:

New Package - Phase2 L1 ParticleFlow
This is a rebase of #30256, with addressed all comments and removed data files.

This PR adds the ParticleFlow based modules are added to the Phase2 L1T Sequence. We distinguish the (a) Lower-level objects, which are inputs to (b) Higher-level objects that are (can be) used in the L1T Menu.

(a) Lower-level objects:

l1ParticleFlow_calo_Task
    pfClustersFromL1EGClusters ,
    pfClustersFromCombinedCaloHCal ,
    pfClustersFromCombinedCaloHF ,
    pfClustersFromHGC3DClusters

l1ParticleFlow_pf_barrel_Task
    pfTracksFromL1TracksBarrel ,
    l1pfProducerBarrel

l1ParticleFlow_pf_hgcal_Task
    pfTracksFromL1TracksHGCal ,
    l1pfProducerHGCal ,
    l1pfProducerHGCalNoTK

l1ParticleFlow_pf_hf_Task
    l1pfProducerHF

Candidates
    l1pfCandidates

(b) Higher-level objects, that are (can be) used in the L1T Menu:

l1PFJetsTask
    ak4PFL1Calo
    ak4PFL1PF
    ak4PFL1Puppi ,
    ak4PFL1CaloCorrected
    ak4PFL1PFCorrected
    ak4PFL1PuppiCorrected

l1PFMets Task
    l1PFMetCalo
    l1PFMetPF
    l1PFMetPuppi

In this PR, we use phase2_trackerV14 modifier to enable in the sequence the phase2 L1T modules that depend on TrackTriggerTracks. This solution effectively means that the central workflows earlier than era Phase2C9 will not execute these modules (neither will TrackTrigger TrackFinderTrack module)

The reason for this is that the Hybrid TrackTriggerTrackFinder at the moment cannot run on geometries earlier that Tracker v14, and are enabled via the same modifier only in eras Phase2C9 and later.
Still to do

Add PF Taus (currently missing from this PR)
Add Phase1 PF jets which should reside in L1Trigger.L1CaloTrigger package

Phase1 PF jets

from L1Trigger.L1CaloTrigger.Phase1L1TJets_cff import *
_phase2_siml1emulator.add(Phase1L1TJetsTask)

However, in this ParticleFlow package we already have the jets ak4PFL1PuppiCorrected that could be used in principle. Although these were not used in the Menu of the L1T TDR, the two have a vary similar performance. The plan is, if possible to include the Phase1 PFJets is time permits, to be fully consistent with the L1T TDR.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 2, 2020

The code-checks are being triggered in jenkins.

@silviodonato
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 2, 2020

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-30510/16712

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 2, 2020

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 2, 2020

A new Pull Request was created by @silviodonato (Silvio Donato) for master.

It involves the following packages:

L1Trigger/Configuration
L1Trigger/Phase2L1ParticleFlow

@cmsbuild, @rekovic, @benkrikler, @kpedro88 can you please review it and eventually sign? Thanks.
@Martin-Grunewald this is something you requested to watch as well.
@silviodonato, @dpiparo you are the release manager for this.

cms-bot commands are listed here

@silviodonato
Copy link
Contributor Author

It crashes at
step2 of 23234.0_TTbar_14TeV+TTbar_14TeV_TuneCP5_2026D49_GenSimHLBeamSpotFull14+DigiFullTrigger_2026D49+RecoFullGlobal_2026D49+HARVESTFullGlobal_2026D49

%MSG-i ThreadStreamSetup:  (NoModuleName) 02-Jul-2020 19:03:04 CEST pre-events
setting # threads 16
setting # streams 16
%MSG
02-Jul-2020 19:03:28 CEST  Initiating request to open file file:step1.root
02-Jul-2020 19:03:32 CEST  Successfully opened file file:step1.root

[...]

Begin processing the 1st record. Run 1, Event 1, LumiSection 1 on stream 0 at 02-Jul-2020 19:13:08.427 CEST
----- Begin Fatal Exception 02-Jul-2020 19:13:21 CEST-----------------------
An exception of category 'ProductNotFound' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 1 stream: 0
   [1] Running path 'FEVTDEBUGHLToutput_step'
   [2] Prefetching for module PoolOutputModule/'FEVTDEBUGHLToutput'
   [3] Prefetching for module L1TCorrectedPFJetProducer/'ak4PFL1CaloCorrected'
   [4] Prefetching for module FastjetJetProducer/'ak4PFL1Calo'
   [5] Prefetching for module L1TPFCandMultiMerger/'l1pfCandidates'
   [6] Prefetching for module L1TPFProducer/'l1pfProducerBarrel'
   [7] Calling method for module L1TkMuonProducer/'L1TkMuons'
Exception Message:
Principal::getByToken: Found zero products matching all criteria
Looking for type: BXVector<l1t::RegionalMuonCand>
Looking for module label: simKBmtfDigis
Looking for productInstanceName: BMTF

   Additional Info:
      [a] If you wish to continue processing events after a ProductNotFound exception,
add "SkipEvent = cms.untracked.vstring('ProductNotFound')" to the "options" PSet in the configuration.

----- End Fatal Exception -------------------------------------------------
02-Jul-2020 19:13:22 CEST  Closed file file:step1.root

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 2, 2020

-1

Tested at: 8738603

CMSSW: CMSSW_11_2_X_2020-07-02-1100
SCRAM_ARCH: slc7_amd64_gcc820

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5c2bb5/7621/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5c2bb5/7621/git-merge-result

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5c2bb5/7621/summary.html

I found follow errors while testing this PR

Failed tests: RelVals

  • RelVals:

When I ran the RelVals I found an error in the following workflows:
20434.0 step2

runTheMatrix-results/20434.0_TTbar_14TeV+TTbar_14TeV_TuneCP5_2026D41_GenSimHLBeamSpotFull14+DigiFullTrigger_2026D41+RecoFullGlobal_2026D41+HARVESTFullGlobal_2026D41/step2_TTbar_14TeV+TTbar_14TeV_TuneCP5_2026D41_GenSimHLBeamSpotFull14+DigiFullTrigger_2026D41+RecoFullGlobal_2026D41+HARVESTFullGlobal_2026D41.log

23234.0 step2
runTheMatrix-results/23234.0_TTbar_14TeV+TTbar_14TeV_TuneCP5_2026D49_GenSimHLBeamSpotFull14+DigiFullTrigger_2026D49+RecoFullGlobal_2026D49+HARVESTFullGlobal_2026D49/step2_TTbar_14TeV+TTbar_14TeV_TuneCP5_2026D49_GenSimHLBeamSpotFull14+DigiFullTrigger_2026D49+RecoFullGlobal_2026D49+HARVESTFullGlobal_2026D49.log

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5c2bb5/7621/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5c2bb5/7621/git-merge-result

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 2, 2020

Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped)

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 3, 2020

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 3, 2020

Pull request #30510 was updated. @cmsbuild, @rekovic, @benkrikler, @kpedro88 can you please check and sign again.

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 3, 2020

+1
Tested at: af539f4
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5c2bb5/7634/summary.html
CMSSW: CMSSW_11_2_X_2020-07-02-2300
SCRAM_ARCH: slc7_amd64_gcc820

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 3, 2020

Comparison job queued.

@silviodonato
Copy link
Contributor Author

merge

@cmsbuild cmsbuild merged commit 9e8441d into cms-sw:master Jul 3, 2020
@silviodonato silviodonato changed the title [WIP] Phase2 L1 ParticleFlow Phase2 L1 ParticleFlow Jul 3, 2020
@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 3, 2020

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

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2784120
  • DQMHistoTests: Total failures: 7730
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2776340
  • DQMHistoTests: Total skipped: 50
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
  • Checked 154 log files, 17 edm output root files, 37 DQM output files

@silviodonato
Copy link
Contributor Author

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

Comparison Summary:

* No significant changes to the logs found

* Reco comparison results: 0 differences found in the comparisons

* DQMHistoTests: Total files compared: 37

* DQMHistoTests: Total histograms compared: 2784120

* DQMHistoTests: Total failures: 7730

* DQMHistoTests: Total nulls: 0

* DQMHistoTests: Total successes: 2776340

* DQMHistoTests: Total skipped: 50

* DQMHistoTests: Total Missing objects: 0

* DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)

* Checked 154 log files, 17 edm output root files, 37 DQM output files

@rekovic please have a look, was it expected?

@rekovic
Copy link
Contributor

rekovic commented Jul 3, 2020

I don't understand this. This PR should be only making extra Phase2 L1T products not used anywhere at the moment, but intended to be used by the future Phase2 HLT.

DQM should not be affected.

@rekovic
Copy link
Contributor

rekovic commented Jul 3, 2020

But perhaps this level of disagreement in comparison (7730 failed / 2776340 successes) is tolerable and is not an indication of true changes introduced.

@kpedro88
Copy link
Contributor

kpedro88 commented Jul 3, 2020

But perhaps this level of disagreement in comparison (7730 failed / 2776340 successes) is tolerable

No, it really isn't.

@silviodonato
Copy link
Contributor Author

I agree with Kevin.
In JetMET / JetValidation / ak4PFJets there is a clear difference @rekovic (https://tinyurl.com/y8ux6qh9)
image

@silviodonato
Copy link
Contributor Author

Diff of dumps of step1 of 5.1_TTbar+TTbarFS+HARVESTFS:

@@ -98048,7 +98048,7 @@ process.ak4PFJets = cms.EDProducer("FastjetJetProducer",
     Rho_EtaMax = cms.double(4.4),
     applyWeight = cms.bool(False),
     doAreaDiskApprox = cms.bool(False),
-    doAreaFastjet = cms.bool(True),
+    doAreaFastjet = cms.bool(False),
     doPUOffsetCorr = cms.bool(False),
     doPVCorrection = cms.bool(False),
     doRhoFastjet = cms.bool(False),
@@ -98483,6 +98483,45 @@ process.ak4PFJetsSK = cms.EDProducer("FastjetJetProducer",
 )

same thing for all other process.akXXPFJetsYY

l1PFMets = cms.Sequence(l1PFMetCalo + l1PFMetPF + l1PFMetPuppi)

from RecoJets.JetProducers.ak4PFJets_cfi import ak4PFJets as _ak4PFJets
_ak4PFJets.doAreaFastjet = False
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the problem is here @rekovic

Copy link
Contributor

@makortel makortel Jul 3, 2020

Choose a reason for hiding this comment

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

This is certainly a problem. Configuration objects should be modified only in the files that define them (and we really should clean up all the bad examples in Configuration/StandardSequences/python).

@tommasoboccali
Copy link

tommasoboccali commented Jul 3, 2020 via email

import FWCore.ParameterSet.Config as cms

from RecoMET.METProducers.pfMet_cfi import pfMet as _pfMet
_pfMet.calculateSignificance = False
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a problem as well.

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

6 participants