Skip to content

TICL version 4: CLUE3D as default Pattern Recognition, Linking #38880

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
rappoccio merged 6 commits into from
Aug 30, 2022
Merged

TICL version 4: CLUE3D as default Pattern Recognition, Linking #38880

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ccabda0
TICL v4 version, CLUE3D as default Pattern Recognition, Trackster/Tra…
felicepantaleo Mar 23, 2022
b2864e7
avoid code duplication, rearrange code
waredjeb Aug 2, 2022
c3f51ca
use TracksterMergeV3 for HLT75e33 wfs
waredjeb Aug 8, 2022
4b296b5
set trackster seedIndex to -1
waredjeb Aug 8, 2022
cb53fdb
bug fix in recordTrackster function
waredjeb Aug 10, 2022
f3b9ad5
Make algo verbosity levels consistent across TICL
Abhirikshma Aug 22, 2022
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
5 changes: 5 additions & 0 deletions Configuration/ProcessModifiers/python/ticl_v3_cff.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import FWCore.ParameterSet.Config as cms

# This modifier is for running TICL v3.

ticl_v3 = cms.Modifier()
24 changes: 24 additions & 0 deletions Configuration/PyReleaseValidation/python/upgradeWorkflowComponents.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,30 @@ def condition(self, fragment, stepList, key, hasHarvest):
upgradeWFs['ticl_FastJet'].step3 = {'--procModifiers': 'fastJetTICL'}
upgradeWFs['ticl_FastJet'].step4 = {'--procModifiers': 'fastJetTICL'}

class UpgradeWorkflow_ticl_v3(UpgradeWorkflow):
def setup_(self, step, stepName, stepDict, k, properties):
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):
return (fragment=="TTbar_14TeV" or 'CloseByP' in fragment or 'Eta1p7_2p7' in fragment) and '2026' in key
upgradeWFs['ticl_v3'] = UpgradeWorkflow_ticl_v3(
steps = [
'RecoGlobal',
'HARVESTGlobal'
],
PU = [
'RecoGlobal',
'HARVESTGlobal'
],
suffix = '_ticl_v3',
offset = 0.203,
Copy link
Contributor

@srimanob srimanob Aug 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

You should update also Configuration/PyReleaseValidation/README.md. It can be done later, no need to do in this PR.

)
upgradeWFs['ticl_v3'].step3 = {'--procModifiers': 'ticl_v3'}
upgradeWFs['ticl_v3'].step4 = {'--procModifiers': 'ticl_v3'}


# Track DNN workflows
class UpgradeWorkflow_trackdnn(UpgradeWorkflow):
def setup_(self, step, stepName, stepDict, k, properties):
Expand Down
52 changes: 52 additions & 0 deletions RecoHGCal/TICL/plugins/LinkingAlgoBase.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
#ifndef RecoHGCal_TICL_LinkingAlgoBase_H__
#define RecoHGCal_TICL_LinkingAlgoBase_H__

#include <vector>
#include "FWCore/ParameterSet/interface/ParameterSetDescription.h"
#include "FWCore/Framework/interface/Event.h"
#include "FWCore/Framework/interface/EventSetup.h"
#include "DataFormats/HGCalReco/interface/Trackster.h"
#include "DataFormats/HGCalReco/interface/TICLCandidate.h"
#include "DataFormats/TrackReco/interface/Track.h"
#include "DataFormats/TrackReco/interface/TrackFwd.h"
#include "DataFormats/MuonReco/interface/Muon.h"
#include "MagneticField/Engine/interface/MagneticField.h"
#include "TrackingTools/GeomPropagators/interface/Propagator.h"
#include "RecoLocalCalo/HGCalRecAlgos/interface/RecHitTools.h"
#include "Geometry/HGCalCommonData/interface/HGCalDDDConstants.h"

namespace edm {
class Event;
class EventSetup;
} // namespace edm

namespace ticl {
class LinkingAlgoBase {
public:
LinkingAlgoBase(const edm::ParameterSet& conf) : algo_verbosity_(conf.getParameter<int>("algo_verbosity")) {}

virtual ~LinkingAlgoBase(){};

virtual void initialize(const HGCalDDDConstants* hgcons,
const hgcal::RecHitTools rhtools,
const edm::ESHandle<MagneticField> bfieldH,
const edm::ESHandle<Propagator> propH) = 0;

virtual void linkTracksters(const edm::Handle<std::vector<reco::Track>> tkH,
const edm::ValueMap<float>& tkTime,
const edm::ValueMap<float>& tkTimeErr,
const edm::ValueMap<float>& tkTimeQual,
const std::vector<reco::Muon>& muons,
const edm::Handle<std::vector<Trackster>> tsH,
std::vector<TICLCandidate>& resultTracksters) = 0;

static void fillPSetDescription(edm::ParameterSetDescription& desc) { desc.add<int>("algo_verbosity", 0); };

enum VerbosityLevel { None = 0, Basic, Advanced, Expert, Guru };
Copy link
Contributor

@jpata jpata Aug 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I would rather prefer not to introduce a module-specific logging configuration and just stick with pure MessageLogger levels.

If you keep this, not only you today, but every future reader and maintainer will have two separate levels to keep in mind and get right, rather than one. If each class has it's own slightly different way of configuring logging, it becomes quite difficult to clean up.

Copy link
Contributor

@felicepantaleo felicepantaleo Aug 16, 2022

Choose a reason for hiding this comment

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

Hi @jpata , thanks for the comment. Eventually it would be nice to have it uniform across CMSSW with Message Logger levels.
I asked the active developers if this existing code is something they are actively using in their debugging workflows, and this module is still under heavy development. Do you mind if we keep it like this until the development of this module becomes maintenance and then we roll back not only linking but also other modules that are using these message levels?

Copy link
Contributor

@jpata jpata Aug 16, 2022

Choose a reason for hiding this comment

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

Anything that is merged is already maintenance and someone other than the immediate developers may have to step in to work on it.
I doubt consistent logging will be implemented in the future, if it's already too troublesome to do now.

If you decide to keep this, you could consider using the same enum across different TICL modules, so at least it's possible to keep self consistent in TICL in the future?
Now I see redefinitions of this VerbosityLevel in PatternRecognitionAlgoBase, HGCGraph, SeedingRegionAlgoBase.
For modules in this PR that take a bool flag debug, you can prefer the enum, so all of them can be configured in the same way.

Copy link
Contributor

@Abhirikshma Abhirikshma Aug 22, 2022

Choose a reason for hiding this comment

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

I have replaced the class wise enums with something that is common for all the TICL modules where this is used


protected:
int algo_verbosity_;
};
} // namespace ticl

#endif
Loading