Skip to content

GNN Track-Tracksters Linking in TICLv5 #49652

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.

Sign up for GitHub

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

Merged
cmsbuild merged 7 commits into from
Jan 12, 2026
Merged

GNN Track-Tracksters Linking in TICLv5 #49652

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7c1112d
GNN Track-Tracksters Linking
Moanwar Dec 16, 2025
d9b6b73
code format
Moanwar Dec 17, 2025
20e27a6
Add workflow .7521 for HLT checks in the ph2_hlt matrix
Moanwar Dec 18, 2025
99236ee
Use TICLGraph for GNN linking
Moanwar Dec 18, 2025
6ddbf85
adding wf .7521 to the ph2_hlt matrix
Moanwar Dec 18, 2025
777302a
fix the indentation
Moanwar Dec 18, 2025
9f2c67c
update the modifier name
Moanwar Jan 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Configuration/ProcessModifiers/python/ticlv5_TrackLinkingGNN_cff.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import FWCore.ParameterSet.Config as cms

from Configuration.ProcessModifiers.ticl_v5_cff import ticl_v5

# This modifier is for running TICL v5 with GNN track-trackster linking.
ticlv5TrackLinkingGNN = cms.Modifier()
ticlv5_TrackLinkingGNN = cms.ModifierChain(ticl_v5, ticlv5TrackLinkingGNN)
34 changes: 33 additions & 1 deletion Configuration/PyReleaseValidation/python/upgradeWorkflowComponents.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -893,7 +893,7 @@ def condition(self, fragment, stepList, key, hasHarvest):
upgradeWFs['ticl_v5'].step2 = {'--procModifiers': 'ticl_v5'}
upgradeWFs['ticl_v5'].step3 = {'--procModifiers': 'ticl_v5'}
upgradeWFs['ticl_v5'].step4 = {'--procModifiers': 'ticl_v5'}

class UpgradeWorkflow_ticl_v5_superclustering(UpgradeWorkflow):
def setup_(self, step, stepName, stepDict, k, properties):
if ('Digi' in step and 'NoHLT' not in step) or ('HLTOnly' in step):
Expand Down Expand Up @@ -1075,6 +1075,38 @@ def condition(self, fragment, stepList, key, hasHarvest):
upgradeWFs['ticl_barrel_CPfromPU'].step3 = {'--procModifiers': 'ticl_barrel,enableCPfromPU'}
upgradeWFs['ticl_barrel_CPfromPU'].step4 = {'--procModifiers': 'ticl_barrel,enableCPfromPU'}

class UpgradeWorkflow_ticlv5_TrackLinkingGNN(UpgradeWorkflow):
def setup_(self, step, stepName, stepDict, k, properties):
if ('Digi' in step and 'NoHLT' not in step) or ('HLTOnly' in step):
stepDict[stepName][k] = merge([self.step2, stepDict[step][k]])
if 'RecoGlobal' in step:
stepDict[stepName][k] = merge([self.step3, stepDict[step][k]])
if 'HARVESTGlobal' in step:
stepDict[stepName][k] = merge([self.step4, stepDict[step][k]])
def condition(self, fragment, stepList, key, hasHarvest):
selected_fragments = ["TTbar_14TeV", "CloseByP", "Eta1p7_2p7", "ZEE_14"]
return any(sf in fragment for sf in selected_fragments) and 'Run4' in key

upgradeWFs['ticlv5_TrackLinkingGNN'] = UpgradeWorkflow_ticlv5_TrackLinkingGNN(
steps = [
'HLTOnly',
'DigiTrigger',
'RecoGlobal',
'HARVESTGlobal'
],
PU = [
'HLTOnly',
'DigiTrigger',
'RecoGlobal',
'HARVESTGlobal'
],
suffix = '_ticlv5_TrackLinkGNN',
offset = 0.211,
)
upgradeWFs['ticlv5_TrackLinkingGNN'].step2 = {'--procModifiers': 'ticlv5_TrackLinkingGNN'}
upgradeWFs['ticlv5_TrackLinkingGNN'].step3 = {'--procModifiers': 'ticlv5_TrackLinkingGNN'}
upgradeWFs['ticlv5_TrackLinkingGNN'].step4 = {'--procModifiers': 'ticlv5_TrackLinkingGNN'}

# L3 Tracker Muon Outside-In reconstruction first
class UpgradeWorkflow_phase2L3MuonsOIFirst(UpgradeWorkflow):
def setup_(self, step, stepName, stepDict, k, properties):
Expand Down
14 changes: 14 additions & 0 deletions HLTrigger/Configuration/python/HLT_75e33/modules/hltTiclCandidate_cfi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,17 @@
useTimingAverage = cms.bool(False)
)

from Configuration.ProcessModifiers.ticlv5_TrackLinkingGNN_cff import ticlv5TrackLinkingGNN
ticlv5TrackLinkingGNN.toModify(
hltTiclCandidate, interpretationDescPSet = cms.PSet(
algo_verbosity = cms.int32(0),
Copy link
Contributor

@mmusich mmusich Dec 18, 2025

Choose a reason for hiding this comment

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

please drop cms type specification where possible.

Copy link
Contributor Author

@Moanwar Moanwar Dec 18, 2025

Choose a reason for hiding this comment

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

I checked this, but removing cms.* here causes a configuration error since the parameter is defined at module level and must retain its CMS type, right ? or I may missing something?

Copy link
Contributor

@mmusich mmusich Dec 18, 2025

Choose a reason for hiding this comment

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

@Moanwar

right ?

wrong. Some of the parameters of interpretationDescPSet are already defined at fillDescriptions level,
see

https://github.com/cms-sw/cmssw-cfipython/blob/37f333a49efeaa3a9a2d5953969664e3a7eaea18/RecoHGCal/TICL/TICLCandidateProducer.py#L23-L31

so at least for those you can drop the cms specification.
The snippet below at least compiles for me.

diff --git a/HLTrigger/Configuration/python/HLT_75e33/modules/hltTiclCandidate_cfi.py b/HLTrigger/Configuration/python/HLT_75e33/modules/hltTiclCandidate_cfi.py
index 94c123436f3..8433b11c034 100644
--- a/HLTrigger/Configuration/python/HLT_75e33/modules/hltTiclCandidate_cfi.py
+++ b/HLTrigger/Configuration/python/HLT_75e33/modules/hltTiclCandidate_cfi.py
@@ -48,16 +48,15 @@ hltTiclCandidate = cms.EDProducer("TICLCandidateProducer",
 )
 
 from Configuration.ProcessModifiers.ticlv5_TrackLinkingGNN_cff import ticlv5TrackLinkingGNN
-ticlv5TrackLinkingGNN.toModify(
-    hltTiclCandidate, interpretationDescPSet = cms.PSet(
-        algo_verbosity = cms.int32(0),
-    cutTk = cms.string('1.48 < abs(eta) < 3.0 && pt > 1. && quality("highPurity") && hitPattern().numberOfLostHits("MISSING_OUTER_HITS") < 5'),
-        onnxTrkLinkingModelFirstDisk = cms.FileInPath('RecoHGCal/TICL/data/ticlv5/onnx_models/TrackLinking_GNN/FirstDiskPropGNN_v0.onnx'),
-        onnxTrkLinkingModelInterfaceDisk = cms.FileInPath('RecoHGCal/TICL/data/ticlv5/onnx_models/TrackLinking_GNN/InterfaceDiskPropGNN_v0.onnx'),
-        inputNames = cms.vstring('x', 'edge_index', 'edge_attr'),
-        output = cms.vstring('output'),
-        delta_tk_ts = cms.double(0.1),
-        thr_gnn = cms.double(0.5),
-        type = cms.string('GNNLink')
-    )
-)
+ticlv5TrackLinkingGNN.toModify(hltTiclCandidate,
+                               interpretationDescPSet = dict(
+                                   algo_verbosity = 0,
+                                   cutTk = '1.48 < abs(eta) < 3.0 && pt > 1. && quality("highPurity") && hitPattern().numberOfLostHits("MISSING_OUTER_HITS") < 5',
+                                   onnxTrkLinkingModelFirstDisk = cms.FileInPath('RecoHGCal/TICL/data/ticlv5/onnx_models/TrackLinking_GNN/FirstDiskPropGNN_v0.onnx'),
+                                   onnxTrkLinkingModelInterfaceDisk = cms.FileInPath('RecoHGCal/TICL/data/ticlv5/onnx_models/TrackLinking_GNN/InterfaceDiskPropGNN_v0.onnx'),
+                                   inputNames = cms.vstring('x', 'edge_index', 'edge_attr'),
+                                   output = cms.vstring('output'),
+                                   delta_tk_ts = cms.double(0.1),
+                                   thr_gnn = cms.double(0.5),
+                                   type = 'GNNLink')
+                               )

Copy link
Contributor Author

@Moanwar Moanwar Dec 18, 2025

Choose a reason for hiding this comment

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

I don’t think this will work. Yes, it may compile, but when running cmsRun it will likely raise an error. From what I can see, the cms.PSet was replaced with a dict, which, as far as I understand, replaces the entire PSet rather than just overriding specific parameters. The original PSet likely contains additional keys that are expected by the plugin. By replacing it with a dict, cmssw then sees parameters that are unknown or unused by the plugin, which leads to configuration errors. Of course, I may be missing something, and thank you for explaining and helping on this. In any case, I also realized that I don’t use algo_verbosity in the GNN plugin, so I removed it from the parameters.

Copy link
Contributor

@makortel makortel Dec 18, 2025

Choose a reason for hiding this comment

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

From what I can see, the cms.PSet was replaced with a dict, which, as far as I understand, replaces the entire PSet rather than just overriding specific parameters.

No, dict only changes the parameters that are specified within the dict. The other parameters of the (already existing) PSet are kept. See https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideCmsDriverEras#Modifying_parameters_of_PSets for more details.

Copy link
Contributor Author

@Moanwar Moanwar Dec 18, 2025

Choose a reason for hiding this comment

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

Sorry, I’m having issues accessing the TWiki today-I’m not sure why.
That said, regarding the parameters: in the original PSet, some parameters are intended to be used by a specific plugin. If we replace the PSet with a dict, parameters that are not used by the new plugin will still be passed to it, which leads to configuration errors. I’m just trying to clearly understand the flow here. I got the error below when using the snippet above, which is why I thought this might be the issue.

[1] Validating configuration of module: class=TICLCandidateProducer label='hltTiclCandidate'
Exception Message:
Illegal parameters found in configuration. The parameters are named:
'cutTk'
'delta_tk_ts_interface'
'delta_tk_ts_layer1'
'timing_quality_threshold'
You could be trying to use parameter names that are not allowed for this plugin or they could be misspelled.

Copy link
Contributor

@makortel makortel Dec 18, 2025

Choose a reason for hiding this comment

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

Ah, I see, thanks for the clarification. Since the plugin gets changed, and presumably any similarity of parameter names is accidental, I agree using a PSet() here instead of a dict() could be easier to understand and maintain.

Technically the dict() syntax allows deletion of parameters (along dict(..., cutTk=None)), but that would make assumptions of the earlier PSet.

Copy link
Contributor

@mmusich mmusich Dec 18, 2025

Choose a reason for hiding this comment

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

That said, regarding the parameters: in the original PSet, some parameters are intended to be used by a specific plugin. If we replace the PSet with a dict, parameters that are not used by the new plugin will still be passed to it, which leads to configuration errors. I’m just trying to clearly understand the flow here.

I think something is being abused by the new plugin. If it cannot use the same structure of the input interpretationDescPSet then it should use an entirely different parameter.

Copy link
Contributor Author

@Moanwar Moanwar Dec 18, 2025

Choose a reason for hiding this comment

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

Well, I wouldn’t say it’s being “abused.” Technically, both plugins have input parameters-one is based on selection cuts, and the other on the GNN model-but in the end, both produce the same kind of output. Having them share the same structure is mostly for simplicity, though it’s not strictly necessary for them to receive exactly the same parameters.

cutTk = cms.string('1.48 < abs(eta) < 3.0 && pt > 1. && quality("highPurity") && hitPattern().numberOfLostHits("MISSING_OUTER_HITS") < 5'),
onnxTrkLinkingModelFirstDisk = cms.FileInPath('RecoHGCal/TICL/data/ticlv5/onnx_models/TrackLinking_GNN/FirstDiskPropGNN_v0.onnx'),
onnxTrkLinkingModelInterfaceDisk = cms.FileInPath('RecoHGCal/TICL/data/ticlv5/onnx_models/TrackLinking_GNN/InterfaceDiskPropGNN_v0.onnx'),
inputNames = cms.vstring('x', 'edge_index', 'edge_attr'),
output = cms.vstring('output'),
delta_tk_ts = cms.double(0.1),
thr_gnn = cms.double(0.5),
type = cms.string('GNNLink')
Copy link
Contributor Author

@Moanwar Moanwar Dec 18, 2025

Choose a reason for hiding this comment

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

The choice of the name "type" is intentional to be the same, as it determines whether the old plugin or the new GNN-based plugin is used. However, I’m wondering if this should compile. I tried quickly and got the following error:

TypeError: type does not already exist, so it can only be set to a CMS Python configuration type

Did it compile successfully on your side?

Copy link
Contributor

@mmusich mmusich Dec 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It did for me. @Moanwar you have to replace cms.PSet with dict first -- of course.

Copy link
Contributor Author

@Moanwar Moanwar Dec 18, 2025

Choose a reason for hiding this comment

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

Hmm, that’s strange. I have tried compiling and running a few times to make sure it works:
runTheMatrix.py -w upgrade -l 29688.211 -j 0
But each time, I get the same error I mentioned above. I’m not sure I understand why this happens, could you please give that a try ? Thanks!

Copy link
Contributor

@mmusich mmusich Dec 18, 2025

Choose a reason for hiding this comment

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

@Moanwar

cmsrel CMSSW_16_0_X_2025-12-17-2300
cd CMSSW_16_0_X_2025-12-17-2300/src/
cmsenv
git cms-merge-topic 49652
# apply patch below [*]
scram b -j 20
runTheMatrix.py -w upgrade -l 29688.211 -j 0

runs fine for me.
hope this helps!

[*]

diff --git a/HLTrigger/Configuration/python/HLT_75e33/modules/hltTiclCandidate_cfi.py b/HLTrigger/Configuration/python/HLT_75e33/modules/hltTiclCandidate_cfi.py
index 307af675b8e..03efda78c33 100644
--- a/HLTrigger/Configuration/python/HLT_75e33/modules/hltTiclCandidate_cfi.py
+++ b/HLTrigger/Configuration/python/HLT_75e33/modules/hltTiclCandidate_cfi.py
@@ -49,13 +49,13 @@ hltTiclCandidate = cms.EDProducer("TICLCandidateProducer",
 
 from Configuration.ProcessModifiers.ticlv5_TrackLinkingGNN_cff import ticlv5TrackLinkingGNN
 ticlv5TrackLinkingGNN.toModify(hltTiclCandidate,
-    interpretationDescPSet = cms.PSet(
+    interpretationDescPSet = dict(
         onnxTrkLinkingModelFirstDisk = cms.FileInPath('RecoHGCal/TICL/data/ticlv5/onnx_models/TrackLinking_GNN/FirstDiskPropGNN_v0.onnx'),
         onnxTrkLinkingModelInterfaceDisk = cms.FileInPath('RecoHGCal/TICL/data/ticlv5/onnx_models/TrackLinking_GNN/InterfaceDiskPropGNN_v0.onnx'),
         inputNames = cms.vstring('x', 'edge_index', 'edge_attr'),
         output = cms.vstring('output'),
         delta_tk_ts = cms.double(0.1),
         thr_gnn = cms.double(0.5),
-        type = cms.string('GNNLink')
+        type = 'GNNLink'
     )
 )

Copy link
Contributor Author

@Moanwar Moanwar Dec 18, 2025

Choose a reason for hiding this comment

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

Ah, thanks a lot @mmusich, That makes sense. I was testing without changing to a dict, which does compile successfully, but then it raises the other error mentioned in cmsRun here: - unless it’s working for you as well?

)
)
1 change: 1 addition & 0 deletions RecoHGCal/TICL/interface/TICLInterpretationAlgoBase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "FWCore/Framework/interface/ConsumesCollector.h"
#include "PhysicsTools/TensorFlow/interface/TensorFlow.h"
#include "DataFormats/Common/interface/MultiSpan.h"
#include "PhysicsTools/ONNXRuntime/interface/ONNXRuntime.h"

namespace edm {
class Event;
Expand Down
Loading