[L1T] PR containing simulation and emulation related to jet mass reconstruction (Phase-2) #47792
Conversation
Adding another commit to squash ahead of PR Squashing 38 previous commits relating to jet mass ahead of opening pull request added logic for calculating jet mass and added this mass to the jet mass value, values printed to screen to ensure they are being set but currently mass not present in ntuples - default value of 0 present changes for calculating jet mass in SW imp Added code to use external seeds for SW implementation removed printing for debug info modified SC jet emulator header file to support jet mass inside l1ct jet type, then changed jet producer plugin to fill the EDM jet with this mass and also changed jet emulator src code makeJet_HW function so it calculates something, though its not correctly the jet mass added mass calculation to emulator following same method as firmware imp, but without LUTs pt_t doesnt have good enough precision for squared values - changed to use floats for now converted eta/phi to from global to relative for jet mass calculation changed emulator imp to use fixed types which give same precision as float with as few bits as possible changed code to use LUTs as implemented in firmware instead of using cpp math functions added code to sort and truncate constituents before passing to jet mass function, uses identical types to firmware introduced preprocessing steps to jet mass function to replicate the format that constituents are received as in firmware Fix eta/phi fiducial check for charged PUPPI candidates Replace the isFiducial(hwEta, hwPhi) call for charged PUPPI candidates to use hwVtxEta and hwVtxPhi instead of hwEta and hwPhi. modified datatypes and methods to handle jet mass Use seed passed in to makeJet_HW as starting point for jet axis. fixes after rebase Cleaning up code and synchronising with p2l1pfp CMSSW fork restored L1CaloTrigger package added SC8 sim jets to the emulator chain fixed a bug with calculating sim jet mass cleaned up how LUTs are produced and synchronised with how it is done in firmware removed old commented out code added floatMass method to gt namespace code-format and code-checks stylistic modifications Fix handling of phi wrap around. changed gt jet equality operator to also check mass populate edmJet using GT jet mass changes for testing using mass2 Put jet mass on second jet word. Add helper function for unpacking two-word jets. made changes to use second word to store jet mass squared, instead of first word for jet mass just code-checks and code-formats changes store unsorted parts not truncated, sorted sparse parts as causes segfault trying to fix seg fault increasing bit width to allow for second word in l1ct::jets fixed bug with LUTs due to compiler dependent rounding vs truncation order Set mass to 17 bits and replace b tag score so that the CT jet is 64 bits. code-checks and code-format added sc8 corrected just to python config Define separate GT Jet format for wide cone jets. removed old commented out code
|
@lrobertshaw, CMSSW_15_1_X branch is closed for direct updates. cms-bot is going to move this PR to master branch. |
|
cms-bot internal usage |
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47792/44372
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47792/44373
|
|
A new Pull Request was created by @lrobertshaw for master. It involves the following packages:
@BenjaminRS, @Moanwar, @cmsbuild, @quinnanm, @srimanob, @subirsarkar can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
test parameters:
|
|
please test |
|
-1 Failed Tests: RelVals |
|
Pull request #47792 was updated. @BenjaminRS, @Moanwar, @cmsbuild, @quinnanm, @srimanob, @subirsarkar can you please check and sign again. |
|
please test |
L1Trigger/Phase2L1ParticleFlow/plugins/L1SeedConePFJetProducer.cc
Outdated
Show resolved
Hide resolved
L1Trigger/Phase2L1ParticleFlow/interface/jetmet/L1SeedConePFJetEmulator.h
Show resolved
Hide resolved
|
+1 Size: This PR adds an extra 16KB to repository Comparison SummarySummary:
|
cmsbuild
added
tests-pending
code-checks-pending
changes-dataformats
and removed
tests-approved
code-checks-approved
labels
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47792/44414
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47792/44415
|
|
Pull request #47792 was updated. @BenjaminRS, @Moanwar, @cmsbuild, @quinnanm, @srimanob, @subirsarkar can you please check and sign again. |
| b_tag_score_t hwBtagScore; | ||
| // b_tag_score_t hwBtagScore; |
There was a problem hiding this comment.
Do we really remove the btag score now from the jets? If so then you can remove the commented out lines here and throughout
| #endif | ||
| #endif |
There was a problem hiding this comment.
Need to have a empty line at the end of file
| @@ -1,6 +1,8 @@ | |||
| #ifndef L1Trigger_Phase2L1ParticleFlow_L1SeedConePFJetEmulator_h | |||
| #define L1Trigger_Phase2L1ParticleFlow_L1SeedConePFJetEmulator_h | |||
|
|
|||
| #define NCONSTITSFW 32 | |||
There was a problem hiding this comment.
Can you add a comment here as to what this is please? (I assume it is the maximum number of constituents?)
| @@ -136,23 +158,28 @@ l1t::PFJet L1SeedConePFJetProducer::makeJet_SW(const std::vector<edm::Ptr<l1t::P | |||
| return jet; | |||
| } | |||
|
|
|||
| // std::vector<l1t::PFJet> | |||
There was a problem hiding this comment.
Remove commented out code
| @@ -229,4 +265,4 @@ void L1SeedConePFJetProducer::fillDescriptions(edm::ConfigurationDescriptions& d | |||
| descriptions.addWithDefaultLabel(desc); | |||
| } | |||
|
|
|||
| DEFINE_FWK_MODULE(L1SeedConePFJetProducer); | |||
| DEFINE_FWK_MODULE(L1SeedConePFJetProducer); | |||
There was a problem hiding this comment.
Add empty line at end of file
| L1SCJetEmu emulator; | ||
| edm::EDGetTokenT<std::vector<l1t::PFCandidate>> l1PFToken; | ||
| l1tpf::corrector corrector; | ||
|
|
||
| std::vector<l1t::PFJet> processEvent_SW(std::vector<edm::Ptr<l1t::PFCandidate>>& parts) const; | ||
| std::vector<l1t::PFJet> processEvent_SW(std::vector<edm::Ptr<l1t::PFCandidate>>& work) const; |
There was a problem hiding this comment.
Can this be parts in order to be consistent with
| @@ -110,15 +180,20 @@ std::vector<L1SCJetEmu::Jet> L1SCJetEmu::emulateEvent(std::vector<Particle>& par | |||
| }); | |||
| if (debug_) { | |||
| dbgCout() << "Seed: " << seed.hwPt << ", " << seed.hwEta << ", " << seed.hwPhi << std::endl; | |||
| std::cout << "N particles : " << particlesInCone.size() << std::endl; | |||
There was a problem hiding this comment.
Should this be dbgCout() instead of std::cout?
| work.end()); | ||
| } | ||
| return jets; | ||
| } | ||
| } |
There was a problem hiding this comment.
Add empty line at end of file
| l1t::PFCandidate seed = *parts.at(0); | ||
| l1t::PFJet L1SeedConePFJetProducer::makeJet_SW(const std::vector<edm::Ptr<l1t::PFCandidate>>& parts, | ||
| const edm::Ptr<l1t::PFCandidate>& seed) const { | ||
| // l1t::PFCandidate seed = *parts.at(0); |
There was a problem hiding this comment.
could this comment be deleted? (L104)
| unsigned nJets; | ||
| bool HW; | ||
| bool debug; | ||
| bool doCorrections; |
There was a problem hiding this comment.
why is it necessary to get rid of the consts?
PR description:
Added logic for calculating jet mass in simulation where no approximations are made. Also added logic for firmware emulation which is bit-accurate to firmware implementation and makes a number of approximations for firmware efficiency. To integrate this work with the various taggers, a number of changes were made to the datatypes which the jet mass is read out to.
PR validation:
Resolution plots produced and compared with existing algorithms which already calculate mass (AK8) to check that the mass is similar. Simulation and emulation checked to match up well. Emulator and firmware match validated by GitLab CI pipelines in CMS-cactus repository and via running tests locally.
Commits squashed into one for the PR, below are all commit messages: