[Phase-2 L1T] Move TauNNIdHW to the externally-built NNPuppiTauModel package#51263
Merged
cmsbuild merged 5 commits intoJul 9, 2026
Merged
Conversation
Replaces the in-tree hls4ml weight arrays (w2/b2/.../w20/b20, global linkage, no namespace) with a call through hls4mlEmulator::Model, loaded at runtime via hls4mlEmulator::ModelLoader from the new cms-hls4ml/ NNPuppiTauModel external. This follows the same pattern already used for JetId/L1TSC4NGJetID, and avoids ELF symbol clashes when multiple hls4ml models are loaded into the same process (cms-sw#49632). - TauNNIdHW takes a std::shared_ptr<hls4mlEmulator::Model> in its constructor; EvaluateNN() becomes a thin prepare_input/predict/ read_result wrapper instead of inlining the dense/relu/sigmoid layer sequence. - L1NNTauProducer gains a ModelLoader member and a configurable tauModelPath parameter (default "NNPuppiTauModel_v1"), mirroring L1TSC4NGJetProducer. - Deletes the now-dead in-tree taus/{defines.h,tau_parameters.h,weights/, nnet_utils/} -- fully superseded by the external package. - Adds a CppUnit test (testNNPuppiTauModel.cppunit.cc) that loads the external model (including two instances at once, the scenario that used to clash on un-namespaced symbols) and sanity-checks predict() output. Co-authored-by: artlbv <arturlbv@gmail.com>
Contributor
|
cms-bot internal usage |
Contributor
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51263/49845
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
Co-authored-by: artlbv <arturlbv@gmail.com>
Contributor
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51263/49846
|
Contributor
|
Pull request #51263 was updated. |
Rename the global input_t/result_t typedefs in TauNNIdHW.h to the distinctive tauinput_t/tauresult_t (matching the inputtype-style naming precedent in L1TSC4NGJetID.h) to avoid reintroducing the kind of unnamespaced global-symbol collision risk this migration is meant to eliminate, and make TauNNIdHW.cc's input-count magic number constexpr. Co-authored-by: artlbv <arturlbv@gmail.com>
Contributor
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51263/49850
|
Contributor
|
A new Pull Request was created by @artlbv for master. It involves the following packages:
@BenjaminRS, @cmsbuild, @quinnanm can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
Contributor
|
please test |
cmsbuild
added
tests-pending
code-checks-pending
and removed
code-checks-approved
labels
Contributor
Author
|
Thanks for the review @quinnanm ! Could you or @BenjaminRS test again? |
Contributor
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51263/50098
|
Contributor
|
Pull request #51263 was updated. @BenjaminRS, @cmsbuild, @quinnanm can you please check and sign again. |
artlbv
added a commit
to artlbv/cmssw
that referenced
this pull request
Jul 7, 2026
Address review comments on cms-sw#51263 (backport to CMSSW_17_0_X)
Contributor
|
please test |
Contributor
|
+1 Size: This PR adds an extra 16KB to repository Comparison SummarySummary:
|
Contributor
|
+l1 |
cmsbuild
added
l1-approved
fully-signed
and removed
l1-pending
pending-signatures
labels
Contributor
|
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 will now be reviewed by the release team before it's merged. @mandrenguyen, @ftenchini, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2) |
Contributor
|
+1 |
This was referenced Jul 9, 2026
cmsbuild
pushed a commit
that referenced
this pull request
Jul 14, 2026
…ce tau typedefs, drop empty dtor, dedupe model name string - Move tauinput_t/tauresult_t typedefs into the L1TauEmu namespace to avoid clashing with other hls4ml model headers, updating all call sites - Replace the empty TauNNIdHW destructor with = default - Store the "NNPuppiTauModel_v1" model name in a single constant in the cppunit test and in the python config instead of repeating the literal Co-authored-by: artlbv <4972492+artlbv@users.noreply.github.com>
cmsbuild
added a commit
that referenced
this pull request
Jul 14, 2026
…CMSSW_17_0_X [Backport of #51263] [Phase-2 L1T] Move TauNNIdHW to the externally-built NNPuppiTauModel package
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
L1Trigger/Phase2L1ParticleFlow's hardware-emulation tau ID/regression path (TauNNIdHW) previously compiled its hls4ml-generated weights in-tree as plain global C arrays (w2,b2, ...,w20,b20). These symbols had no namespace and external linkage, so when other in-tree or externally-loaded hls4ml models were linked into the same process, ELF symbol interposition could silently clobber NNPuppiTau's weights with another model's weights of the same name — the bug class behind #49632.This PR fixes that by moving NNPuppiTau onto the same "externals" pattern already used by
AXOL1TL,L1TSC4NGJetModel, and (within this same package)JetId/L1TSC4NGJetID: all hls4ml-generated code (weights, layer configs, the dense/activation call sequence) now lives in a new standalone, versioned, externally-built shared library, cms-hls4ml/NNPuppiTauModel, with every symbol except the fixedextern "C" create_model()/destroy_model()factory pair wrapped in a per-model+version C++ namespace (hls4ml_nnpuppitaumodel_v1). CMSSW now loads the model at runtime viahls4mlEmulator::ModelLoader/hls4mlEmulator::Model, never touching the raw weight symbols directly.This is part of a 3-PR migration:
cmsdistPR — spec + toolfile so SCRAM builds/installs the new external (Add NNPuppiTauModel external spec and toolfile cmsdist#10665)cmsswchanges to consume it.Changes
TauNNIdHWconstructor now takes astd::shared_ptr<hls4mlEmulator::Model>instead of reading in-tree weight headers;EvaluateNN()becomes a thinprepare_input/predict/read_resultwrapper instead of inlining the dense/relu/sigmoid layer sequence.L1NNTauProducergains anhls4mlEmulator::ModelLoadermember and a new configurabletauModelPathparameter (default"NNPuppiTauModel_v1"), mirroringL1TSC4NGJetProducer.interface/taus/{defines.h, tau_parameters.h, weights/, nnet_utils/}— fully superseded by the external package.testNNPuppiTauModel.cppunit.cc) that loads the external model — including two instances at once, the exact scenario that used to clash on un-namespaced symbols — and sanity-checkspredict()output.Dependencies
Requires the
cmsdistexternal forNNPuppiTauModelto be available in the IB before this can build (see linked cmsdist PR).PR validation
Code compiles against the
NNPuppiTauModel_v1external loaded viahls4mlEmulator::ModelLoader. New unit test exercises model load (including multi-instance) and a basicpredict()range check. No numeric regression expected — the layer sequence and weights are an unmodified, mechanical extraction from the previous in-tree code; full RelVal numeric comparison against currentmasteroutput recommended once cms-bot picks this up.Follow-up:
I ran the L1Nano (+HLT) workflow from #51004 (
runTheMatrix.py -l 34434.774 -w upgrade --ibeos -i all) for both the latest IB baseline and the PR build, then compared allL1nnPuppiTau_*branches (pt, chargedIso/NN score, fullIso, dXY, id, and allpass/failflags) along with all 40 L1_* P2GT emulator decision bits for 100 events and did not find any mismatches anywhere.