[L1T] L1TSC4NGJetModel (Phase 2) #47798

Chriisbrown · 2025-04-07T16:39:06Z

PR description:

This PR adds the v0.0.0 Level-1 Trigger SeededCone 4 Next Gen Jet Model to CMSSW. This model tags seededcone 4 jets into 8 different classes uds, g, b, c, tau+, tau-, e, mu and regresses a pT correction for the jet.

This PR also includes changes to the jet data format to include the additional tag scores.
There are minor changes to the PF datatypes to add proper functionality for dxy

The model training is documented here
This model is specifically from this release of the code

This model's training artefacts; firmware, model file and performance plots are located here
The cms-hls4ml model file tagged v0.0.0 is found here
The in progress DPS note is found here

This PR relies on an addition to the cms-dist externals, with the open PR cms-sw/cmsdist#9778

PR validation:

Testing of the code happens automatically as part of the model training CI, artefacts of this are found here

Standard code style checks have been performed as has a phase-2 Level-1 trigger cmsDriver test as documented here

cmsbuild · 2025-04-07T16:39:33Z

cms-bot internal usage

cmsbuild · 2025-04-15T10:05:01Z

+1

Size: This PR adds an extra 16KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4b45cb/45549/summary.html
COMMIT: a6c8e7c
CMSSW: CMSSW_15_1_X_2025-04-14-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/47798/45549/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 16 lines to the logs
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 51
DQMHistoTests: Total histograms compared: 4027853
DQMHistoTests: Total failures: 23
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 4027810
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
Checked 220 log files, 188 edm output root files, 51 DQM output files
TriggerResults: no differences found

quinnanm · 2025-04-15T20:38:12Z

L1Trigger/Phase2L1ParticleFlow/plugins/L1TSC4NGJetProducer.cc

+      fMaxEta_(cfg.getParameter<double>("maxEta")),
+      fMaxJets_(cfg.getParameter<int>("maxJets")),
+      fNParticles_(cfg.getParameter<int>("nParticles")),
+      loader(hls4mlEmulator::ModelLoader(cfg.getParameter<string>("l1tSC4NGJetModelPath"))) {


Just a note: in Run 3 we get the model names from the menu, but we should generally discuss how we want to do this in Phase 2

quinnanm · 2025-04-15T20:38:58Z

L1Trigger/Phase2L1ParticleFlow/plugins/L1TSC4NGJetProducer.cc

+  std::vector<l1ct::JetTagClass> classes_;
+
+  hls4mlEmulator::ModelLoader loader;
+  std::shared_ptr<hls4mlEmulator::Model> model;


could you add a logerror in case the model is not loaded properly?

Added a error throw when the model is not found

quinnanm · 2025-04-15T20:41:55Z

L1Trigger/Phase2L1ParticleFlow/src/L1TSC4NGJet.cc

+    fId_.get()[i0] = iParts[i0]->id();
+  }
+  setNNVectorVar();
+  return EvaluateNNFixed();


general comment: would it be useful to add some log error/debug lines in case the nn evaluation doesnt work as expected?

Added debugging to model input and output so the NN evaluation can be properly debugged

quinnanm · 2025-04-15T20:42:36Z

L1Trigger/Phase2L1ParticleFlow/src/l1-converters/tkinput_ref.cpp

@@ -363,11 +369,29 @@ void l1ct::TrackInputEmulator::configPhi(int bits) {

 float l1ct::TrackInputEmulator::floatZ0(ap_int<12> z0) const { return z0Scale_ * toFloat_(z0); }

+// float l1ct::TrackInputEmulator::floatDxy(ap_int<13> dxy) const { return dxyScale_ * toFloat_(dxy); }
+float l1ct::TrackInputEmulator::floatDxy(ap_int<13> dxy) const {
+  // return dxyScale_ * toFloat_(dxy);


can this comment be removed? (L372, L374)

quinnanm · 2025-04-15T20:42:46Z

L1Trigger/Phase2L1ParticleFlow/src/l1-converters/tkinput_ref.cpp

 l1ct::z0_t l1ct::TrackInputEmulator::convZ0(ap_int<12> z0) const {
  int offs = z0 >= 0 ? z0OffsPos_ : z0OffsNeg_;
  return (z0.to_int() * z0Mult_ + offs) >> z0BitShift_;
 }

+l1ct::dxy_t l1ct::TrackInputEmulator::convDxy(ap_int<13> dxy) const {
+  int offs = dxy >= 0 ? dxyOffsPos_ : dxyOffsNeg_;
+  // return (dxy.to_int() * dxyMult_ + offs) >> dxyBitShift_;


can this comment be removed?

quinnanm · 2025-04-15T20:45:48Z

DataFormats/L1TParticleFlow/interface/datatypes.h

@@ -34,7 +34,7 @@ namespace l1ct {
  typedef ap_ufixed<10, 5, AP_TRN, AP_SAT> hoe_t;
  typedef ap_uint<4> redChi2Bin_t;
  typedef ap_fixed<10, 1, AP_RND_CONV, AP_SAT> id_score_t;  // ID score to be between -1 (background) and 1 (signal)
-  typedef ap_ufixed<10, 1, AP_RND, AP_SAT> b_tag_score_t;   // result_t from the NN is still apx_fixed<16,6>
+  typedef ap_ufixed<8, 1, AP_RND_CONV, AP_SAT> jet_tag_score_t;  // 8 bit jet jet probability from 0 to 1


Just something to keep in mind: we found with axol1tl that the score type is subject to change with different model versions, so we are thinking about adding a template function to the base cms-hls4ml emulator class to get the result/score type so that the score type does not need to be hardcoded into the emulator. just something to look out for and perhaps modify in the future

quinnanm · 2025-04-15T20:46:05Z

DataFormats/L1TParticleFlow/interface/datatypes.h

@@ -144,7 +144,9 @@ namespace l1ct {
    constexpr float INTPT_LSB = 0.25;
    constexpr float ETAPHI_LSB = M_PI / INTPHI_PI;
    constexpr float Z0_LSB = 0.05;
-    constexpr float DXY_LSB = 0.05;
+    //constexpr float DXY_LSB = 0.05;


could this comment be removed?

quinnanm · 2025-04-18T15:18:21Z

L1Trigger/Phase2L1ParticleFlow/src/l1-converters/tkinput_ref.cpp

+  dxyMult_ = std::round(scale * (1 << bits));
+  switch (encoding_) {
+    case Encoding::Stepping:
+      dxyOffsPos_ = std::round(+scale * 0.5 * (1 << bits) + 0.5 * (1 << bits));
+      dxyOffsNeg_ = std::round(-scale * 0.5 * (1 << bits) + 0.5 * (1 << bits));
+      break;
+    case Encoding::Biased:
+      dxyOffsPos_ = std::round(+scale * 0.5 * (1 << bits) + 0.5 * (1 << bits));
+      dxyOffsNeg_ = std::round(+scale * 0.5 * (1 << bits) + 0.5 * (1 << bits));
+      break;
+    case Encoding::Unbiased:


some comments explaining what is being done here might be useful

Added comment explaining the function and the variables it sets

BenjaminRS

Hi Chris, thanks for this PR. I have some comments on the code.

BenjaminRS · 2025-04-16T12:43:38Z

DataFormats/L1TParticleFlow/interface/datatypes.h

@@ -19,7 +19,7 @@ namespace l1ct {
  typedef ap_int<11> glbphi_t;
  typedef ap_int<5> vtx_t;
  typedef ap_int<10> z0_t;         // 40cm / 0.1
-  typedef ap_int<8> dxy_t;         // tbd
+  typedef ap_uint<8> dxy_t;        // tbd


In the comment: why is this tbd?

Changed from tbd to an explanation of the variable

BenjaminRS · 2025-04-16T12:45:00Z

DataFormats/L1TParticleFlow/interface/datatypes.h

+    constexpr float DXY_LSB = 0.00390625;
+    constexpr float DXYSQRT_LSB = 0.015625;


Is it possible to add comments explaining these values please?

Added comments explaining where these values come from

BenjaminRS · 2025-04-16T12:46:25Z

DataFormats/L1TParticleFlow/interface/datatypes.h

    inline float floatPuppiW(puppiWgt_t puppiw) { return puppiw.to_float(); }
    inline float floatIso(iso_t iso) { return iso.to_float(); }
    inline float floatSrrTot(srrtot_t srrtot) { return srrtot.to_float() / SRRTOT_SCALE; };
    inline float floatMeanZ(meanz_t meanz) { return meanz + MEANZ_OFFSET; };
    inline float floatHoe(hoe_t hoe) { return hoe.to_float(); };
    inline float floatIDScore(id_score_t score) { return score.to_float(); };
-    inline float floatBtagScore(b_tag_score_t b_tag_score) { return b_tag_score.to_float(); }


Is it correct to remove the b-tag score completely?

We now wrap everything up in the float ID score which supersedes the BtagScore

BenjaminRS · 2025-04-16T12:49:13Z

DataFormats/L1TParticleFlow/interface/datatypes.h

@@ -169,20 +171,20 @@ namespace l1ct {
    inline float floatEta(glbeta_t eta) { return eta.to_float() * ETAPHI_LSB; }
    inline float floatPhi(glbphi_t phi) { return phi.to_float() * ETAPHI_LSB; }
    inline float floatZ0(z0_t z0) { return z0.to_float() * Z0_LSB; }
-    inline float floatDxy(dxy_t dxy) { return dxy.to_float() * DXY_LSB; }
+    inline float floatDxy(dxy_t dxy) { return dxy.to_float() * DXYSQRT_LSB; }


Is it correct to have this using DXYSQRT_LSB and in

cmssw/DataFormats/L1TParticleFlow/interface/datatypes.h

Line 187 in a6c8e7c

inline dxy_t makeDxy(float dxy) { return dxy_t(std::round(dxy / DXY_LSB)); }

using DXY_LSB?

This has been changes at L187 to use DXYSQRT_LSB

BenjaminRS · 2025-04-16T12:57:46Z

DataFormats/L1TParticleFlow/interface/gt_datatypes.h

-    inline bool operator==(const Jet &other) const { return valid == other.valid && z0 == other.z0 && v3 == other.v3; }
+    inline bool operator==(const Jet &other) const {
+      bool eq = valid == other.valid && z0 == other.z0 && v3 == other.v3;
+      for (unsigned i = 0; i < NTagFields; i++) {


Could simplify here and L109 and L136 with

Suggested change

for (unsigned i = 0; i < NTagFields; i++) {

for (auto i:NTagFields) {

Not changed as NTagFields is a single integer value so there wasn't a clean auto loop version

As an FYI you can include the ranges library and then use:

for (auto i : std::views::iota(0, static_cast<int>(NTagFields))){

But it is up to you if you want to change these or not

BenjaminRS · 2025-04-17T16:38:15Z

L1Trigger/Phase2L1ParticleFlow/interface/L1TSC4NGJetID.h

+#include "L1Trigger/Phase2L1ParticleFlow/interface/jetmet/L1SeedConePFJetEmulator.h"
+
+//HLS4ML compiled emulator modeling
+#include <string>


Already included on

cmssw/L1Trigger/Phase2L1ParticleFlow/interface/L1TSC4NGJetID.h

Line 4 in a6c8e7c

#include <string>

BenjaminRS · 2025-04-17T16:38:53Z

L1Trigger/Phase2L1ParticleFlow/interface/L1TSC4NGJetID.h

+class L1TSC4NGJetID {
+public:
+  L1TSC4NGJetID(const std::shared_ptr<hls4mlEmulator::Model> model, int iNParticles);
+  ~L1TSC4NGJetID() = default;


Not needed and removed

BenjaminRS · 2025-04-17T19:54:23Z

L1Trigger/Phase2L1ParticleFlow/plugins/L1TSC4NGJetProducer.cc

+#include <string>
+#include "ap_fixed.h"
+#include "hls4ml/emulator.h"


No need for these as they are already included in

cmssw/L1Trigger/Phase2L1ParticleFlow/interface/L1TSC4NGJetID.h

Lines 12 to 14 in a6c8e7c

#include <string>

#include "ap_fixed.h"

#include "hls4ml/emulator.h"

BenjaminRS · 2025-04-17T20:01:01Z

L1Trigger/Phase2L1ParticleFlow/plugins/L1TSC4NGJetProducer.cc

+#include "ap_fixed.h"
+#include "hls4ml/emulator.h"
+
+using namespace l1t;


Can we avoid this please? I see l1t::PFJet is used below anyway

BenjaminRS · 2025-04-17T20:13:01Z

L1Trigger/Phase2L1ParticleFlow/plugins/L1TSC4NGJetProducer.cc

+  }
+  std::sort(taggedJets.begin(), taggedJets.end(), [](l1t::PFJet a, l1t::PFJet b) { return (a.pt() > b.pt()); });
+
+  std::unique_ptr<l1t::PFJetCollection> taggedJetsCollection(new l1t::PFJetCollection);


Should use std::make_unique instead of the new

PR code review 1

cmsbuild · 2025-04-24T07:28:12Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47798/44599

Found files with invalid states:
- L1Trigger/Phase2L1ParticleFlow/python/l1tSC4NGJetProducer_cff.py:
  - Added: fc596e0
  - Deleted: 41123ee
- L1Trigger/Phase2L1ParticleFlow/python/L1MultiJetProducer_cff.py:
  - Added: 649527c
  - Modified: 8b03c4b
  - Deleted: fc596e0
- L1Trigger/Phase2L1ParticleFlow/src/MultiJetId.cc:
  - Added: 649527c
  - Deleted: fc596e0
- L1Trigger/Phase2L1ParticleFlow/interface/MultiJetId.h:
  - Added: 649527c
  - Deleted: fc596e0
- L1Trigger/Phase2L1ParticleFlow/plugins/L1MultiJetProducer.cc:
  - Added: 649527c
  - Modified: 8b03c4b, 17e69ec
  - Deleted: fc596e0
There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/L1TParticleFlow/interface/PFJet.h modified in PR(s): [L1T] PR containing simulation and emulation related to jet mass reconstruction (Phase-2) #47792
- File DataFormats/L1TParticleFlow/interface/datatypes.h modified in PR(s): [L1T] PR containing simulation and emulation related to jet mass reconstruction (Phase-2) #47792
- File DataFormats/L1TParticleFlow/interface/gt_datatypes.h modified in PR(s): [L1T] PR containing simulation and emulation related to jet mass reconstruction (Phase-2) #47792
- File DataFormats/L1TParticleFlow/interface/jets.h modified in PR(s): [L1T] PR containing simulation and emulation related to jet mass reconstruction (Phase-2) #47792
- File DataFormats/L1TParticleFlow/src/classes_def.xml modified in PR(s): [L1T] PR containing simulation and emulation related to jet mass reconstruction (Phase-2) #47792
- File L1Trigger/Configuration/python/SimL1Emulator_cff.py modified in PR(s): [L1T] PR containing simulation and emulation related to jet mass reconstruction (Phase-2) #47792
- File L1Trigger/Phase2L1ParticleFlow/python/l1pfJetMet_cff.py modified in PR(s): [L1T] PR containing simulation and emulation related to jet mass reconstruction (Phase-2) #47792

cmsbuild · 2025-04-24T07:28:37Z

Pull request #47798 was updated. @BenjaminRS, @Moanwar, @cmsbuild, @quinnanm, @srimanob, @subirsarkar can you please check and sign again.

BenjaminRS · 2025-04-24T14:20:31Z

test parameters:

workflow = 29634.78
pull_request = Add L1TSC4NGJetModel cmsdist#9778
relvals_opt = --what upgrade

BenjaminRS · 2025-04-24T14:21:08Z

please test

cmsbuild · 2025-04-24T17:09:07Z

+1

Size: This PR adds an extra 16KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4b45cb/45702/summary.html
COMMIT: f7583c7
CMSSW: CMSSW_15_1_X_2025-04-24-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/47798/45702/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 15 lines to the logs
Reco comparison results: 8 differences found in the comparisons
DQMHistoTests: Total files compared: 51
DQMHistoTests: Total histograms compared: 4024789
DQMHistoTests: Total failures: 19
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 4024750
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
Checked 220 log files, 188 edm output root files, 51 DQM output files
TriggerResults: no differences found

BenjaminRS · 2025-04-24T18:30:59Z

+l1

cerminar and others added 21 commits April 2, 2025 15:10

update geometry

8cb5cc7

fix decoded track collection for missing tracks

930b3c9

Add multijet to 14_2

649527c

add dxy changes

974b131

add dxy datatypes change

954f86d

Modify output product of multi-class jet tagger

8b03c4b

Remove the cout

17e69ec

Change model name and jet format

fc596e0

Fix L1TSC4NGJet.cc to proper inputs

87a9c95

Update to proper inputs

ba06089

Add jet constituents + set hw pt

2ed0807

delete the btag dataypes

723e833

make hls compatible

dde7f13

Update jets.cpp to array data structure

df33708

add types to enum

1a1299a

Add getters for tag score argument class

ea2cbd6

Update jet class version

4d6d001

fix inconsistencies

b3ebf4a

Code formats

425b35f

Rework configs

41123ee

Change default ordering

4349bb4

cmsbuild added this to the CMSSW_15_1_X milestone Apr 7, 2025

cmsbuild added l1-pending pending-signatures tests-pending orp-pending upgrade-pending code-checks-pending changes-dataformats labels Apr 7, 2025

cmsbuild added the tests-started label Apr 15, 2025

cmsbuild added tests-approved and removed tests-started labels Apr 15, 2025

quinnanm reviewed Apr 18, 2025

View reviewed changes

BenjaminRS reviewed Apr 18, 2025

View reviewed changes

Chriisbrown added 4 commits April 22, 2025 14:05

add pattern files

47d44a9

minor changes

393cbda

comments 1

865ea44

code checks

bd2cdc8

Chriisbrown mentioned this pull request Apr 24, 2025

PR code review 1 CMS-L1T-Jet-Tagging/cmssw#5

Merged

PR code review 1

f7583c7

PR code review 1

cmsbuild added tests-pending code-checks-pending and removed tests-approved code-checks-approved labels Apr 24, 2025

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 24, 2025

cmsbuild added tests-started and removed tests-pending labels Apr 24, 2025

cmsbuild added tests-approved and removed tests-started labels Apr 24, 2025

cmsbuild added l1-approved and removed l1-pending labels Apr 24, 2025

		constexpr float DXY_LSB = 0.00390625;
		constexpr float DXYSQRT_LSB = 0.015625;

	for (unsigned i = 0; i < NTagFields; i++) {
	for (auto i:NTagFields) {

	#include <string>
	#include "ap_fixed.h"
	#include "hls4ml/emulator.h"

[L1T] L1TSC4NGJetModel (Phase 2) #47798

Are you sure you want to change the base?

[L1T] L1TSC4NGJetModel (Phase 2) #47798

Conversation

Chriisbrown commented Apr 7, 2025

PR description:

PR validation:

cmsbuild commented Apr 7, 2025 • edited Loading

cmsbuild commented Apr 15, 2025

Comparison Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenjaminRS left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmsbuild commented Apr 24, 2025

cmsbuild commented Apr 24, 2025

BenjaminRS commented Apr 24, 2025

BenjaminRS commented Apr 24, 2025

cmsbuild commented Apr 24, 2025

Comparison Summary

BenjaminRS commented Apr 24, 2025

cmsbuild commented Apr 7, 2025 •

edited

Loading