Skip to content
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

Embedding update for run3 #47299

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Embedding update for run3 #47299

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1937801
remove not necessary merging steps
Oct 2, 2024
878160f
Introduce dummy DetectorStateFilter for embedding, which is passed by…
moritzmolch Oct 8, 2024
a059d00
Replace original DetectorStateFilter in HLT path with EmbeddingDetect…
moritzmolch Oct 8, 2024
24ff9bc
remove not necessary merging steps
Oct 2, 2024
69a3e80
change muon HLT trigger in selection decission
Feb 7, 2025
064da79
Merge branch 'hlt_detector_state_filter_fix' into embedding_update_fo…
Feb 7, 2025
d27b7fe
Simplify implementation of dummy detector state filters (#4)
moritzmolch Feb 10, 2025
c55bde0
readd stuff from run2 with era modifier
Feb 12, 2025
ac65461
change Trigger conditions in selection step and add comments
Feb 13, 2025
36d2593
add a unit test for 2022postEE data
Feb 13, 2025
a3c5795
implement the era modifiers, for run2 only, the correct way
Feb 14, 2025
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
4 changes: 4 additions & 0 deletions TauAnalysis/MCEmbeddingTools/plugins/BuildFile.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
<use name="DataFormats/RPCRecHit"/>
<use name="DataFormats/EcalRecHit"/>
<use name="DataFormats/HcalRecHit"/>
<use name="DataFormats/Common"/>
<use name="DataFormats/FEDRawData"/>
<use name="DataFormats/OnlineMetaData"/>
<use name="DataFormats/Scalers"/>
<use name="TrackingTools/TrackAssociator"/>
<use name="CondFormats/BeamSpotObjects"/>
<use name="SimDataFormats/GeneratorProducts"/>
Expand Down
109 changes: 109 additions & 0 deletions TauAnalysis/MCEmbeddingTools/plugins/EmbeddingDetectorStateFilter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
// -*- C++ -*-
//
// Package: TauAnalysis/MCEmbeddingTools
// Class: EmbeddingDetecorStateFilter
//
/**\class EmbeddingDetectorStateFilter EmbeddingDetectorStateFilter.cc TauAnalysis/MCEmbeddingTools/plugins/EmbeddingDetectorStateFilter.cc

Description: Dummy implementation of the DetectorStateFilter class from DQM/TrackerCommon/plugins/DetectorStateFilter.cc

Implementation:
[Notes on implementation]
*/

#include "DataFormats/Common/interface/Handle.h"
#include "DataFormats/FEDRawData/interface/FEDNumbering.h"
#include "DataFormats/FEDRawData/interface/FEDRawData.h"
#include "DataFormats/FEDRawData/interface/FEDRawDataCollection.h"
#include "DataFormats/Scalers/interface/DcsStatus.h"
#include "DataFormats/OnlineMetaData/interface/DCSRecord.h"
#include "FWCore/Framework/interface/Event.h"
#include "FWCore/Framework/interface/stream/EDFilter.h"
#include "FWCore/MessageLogger/interface/MessageLogger.h"
#include "FWCore/ParameterSet/interface/ParameterSet.h"
#include "FWCore/ParameterSet/interface/ConfigurationDescriptions.h"
#include "FWCore/ParameterSet/interface/ParameterSetDescription.h"

#include <type_traits> // for std::is_same


class EmbeddingDetectorStateFilter : public edm::stream::EDFilter<> {
public:
EmbeddingDetectorStateFilter(const edm::ParameterSet&);
~EmbeddingDetectorStateFilter() override;
static void fillDescriptions(edm::ConfigurationDescriptions& descriptions);

private:
bool filter(edm::Event&, edm::EventSetup const&) override;

const bool verbose_;
uint64_t nEvents_, nSelectedEvents_;
bool detectorOn_;
const std::string detectorType_;
const std::vector<std::string> combinations_; // Vector of strings specifying accepted combinations
const edm::EDGetTokenT<DcsStatusCollection> dcsStatusLabel_;
const edm::EDGetTokenT<DCSRecord> dcsRecordToken_;
};


/**
* Dummy implementation of the DetectorStateFilter class in DQM/TrackerCommon/plugins/DetectorStateFilter.cc.
*
* This implementation emulates the branch of the DetectorStateFilter class for MC events. Embedding events are data events, but the detector is simulated. This means that this module fails as the detector state of the simulated detector is tried to read out. This dummy implementation aims to mitigate the detector read-out so that this module, which is part of some HLT sequences, is passed.
*/
EmbeddingDetectorStateFilter::EmbeddingDetectorStateFilter(const edm::ParameterSet& pset)
: verbose_(pset.getUntrackedParameter<bool>("DebugOn", false)),
detectorType_(pset.getUntrackedParameter<std::string>("DetectorType", "sistrip")),
combinations_(pset.getUntrackedParameter<std::vector<std::string>>("acceptedCombinations")),
dcsStatusLabel_(consumes<DcsStatusCollection>(
pset.getUntrackedParameter<edm::InputTag>("DcsStatusLabel", edm::InputTag("scalersRawToDigi")))),
dcsRecordToken_(consumes<DCSRecord>(
pset.getUntrackedParameter<edm::InputTag>("DCSRecordLabel", edm::InputTag("onlineMetaDataDigis")))) {
nEvents_ = 0;
nSelectedEvents_ = 0;
detectorOn_ = false;
}


/**
* Destructor of the EmbeddingDetectorStateFilter class.
*/
EmbeddingDetectorStateFilter::~EmbeddingDetectorStateFilter() = default;


/**
* Dummy filter implementation, in which each event passes the filter.
*/
bool EmbeddingDetectorStateFilter::filter(edm::Event& evt, edm::EventSetup const& es)
{
// implement the branch for MC events from the original DetectorStateFilter implementation
nEvents_++;
detectorOn_ = true;
nSelectedEvents_++;
if (verbose_) {
edm::LogInfo("DetectorStatusFilter") << "Total MC Events " << nEvents_ << " Selected Events " << nSelectedEvents_
<< " Detector States " << detectorOn_ << std::endl;
}
return detectorOn_;
}


/**
* Fill 'descriptions' with the allowed parameters for the module.
*/
void EmbeddingDetectorStateFilter::fillDescriptions(edm::ConfigurationDescriptions& descriptions) {
edm::ParameterSetDescription desc;
desc.setComment("filters on the HV status of the Tracker (either pixels or strips)");
desc.addUntracked<bool>("DebugOn", false)->setComment("activates debugging");
desc.addUntracked<std::string>("DetectorType", "sistrip")->setComment("either strips or pixels");
desc.addUntracked<std::vector<std::string>>("acceptedCombinations", {});
desc.addUntracked<edm::InputTag>("DcsStatusLabel", edm::InputTag("scalersRawToDigi"))
->setComment("event data for DCS (Run2)");
desc.addUntracked<edm::InputTag>("DCSRecordLabel", edm::InputTag("onlineMetaDataDigis"))
->setComment("event data for DCS (Run3)");
descriptions.add("_detectorStateFilter", desc);
}


#include "FWCore/Framework/interface/MakerMacros.h"
DEFINE_FWK_MODULE(EmbeddingDetectorStateFilter);
2 changes: 1 addition & 1 deletion TauAnalysis/MCEmbeddingTools/python/SelectingProcedure_cff.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
hltResults = cms.InputTag("TriggerResults","","HLT"),
l1tResults = cms.InputTag(""),
throw = cms.bool(False),
triggerConditions = cms.vstring("HLT_Mu17_TrkIsoVVL_Mu8_TrkIsoVVL_DZ_v* OR HLT_Mu17_TrkIsoVVL_Mu8_TrkIsoVVL_DZ_Mass8_v*") # from 2017 on (up to Run 3, it seems)
triggerConditions = cms.vstring("HLT_Mu17_TrkIsoVVL_Mu8_TrkIsoVVL_DZ_Mass3p8_v* OR HLT_Mu17_TrkIsoVVL_Mu8_TrkIsoVVL_DZ_Mass8_v*") # from 2017 on (up to Run 3, it seems)
)

run2_HLTconditions_2016.toModify(doubleMuonHLTTrigger,
Expand Down
95 changes: 22 additions & 73 deletions TauAnalysis/MCEmbeddingTools/python/customisers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

# Various set of customise functions needed for embedding
import FWCore.ParameterSet.Config as cms

from PhysicsTools.NanoAOD.common_cff import ExtVar

################################ Customizer for skimming ###########################
Expand Down Expand Up @@ -61,6 +60,7 @@ def __init__(
module_name="generalTracks", manipulator_name="Track", steps=["SIM", "MERGE"]
)
)
# needed by the PFLinker:particleFlow to create MuonProducer:muons
to_bemanipulate.append(
module_manipulate(
module_name="cosmicsVetoTracksRaw", manipulator_name="Track", steps=["SIM", "MERGE"]
Expand All @@ -80,27 +80,15 @@ def __init__(
steps=["SIM", "MERGE"],
)
)
to_bemanipulate.append(
module_manipulate(
module_name="conversionStepTracks",
manipulator_name="Track",
steps=["SIM", "MERGE"],
)
)
# This is needed by displacedMuonReducedTrackExtras which is specifically added in the merging_step
to_bemanipulate.append(
module_manipulate(
module_name="displacedTracks",
manipulator_name="Track",
steps=["SIM", "MERGE"],
)
)
to_bemanipulate.append(
module_manipulate(
module_name="ckfInOutTracksFromConversions",
manipulator_name="Track",
steps=["SIM", "MERGE"],
)
)
# This is needed by the conversion producer which is run by the highlevelreco task
to_bemanipulate.append(
module_manipulate(
module_name="ckfOutInTracksFromConversions",
Expand All @@ -114,6 +102,7 @@ def __init__(
module_name="muons1stStep", manipulator_name="Muon", steps=["SIM", "MERGE"]
)
)
# needed by MuIsoDepositCopyProducer/'muIsoDepositTkDisplaced'
to_bemanipulate.append(
module_manipulate(
module_name="displacedMuons1stStep", manipulator_name="Muon", steps=["SIM", "MERGE"]
Expand All @@ -133,6 +122,7 @@ def __init__(
steps=["SIM", "MERGE"],
)
)
# needed by the PFLinker:particleFlow to create MuonProducer:muons
to_bemanipulate.append(
module_manipulate(
module_name="particleFlowTmp",
Expand All @@ -155,25 +145,12 @@ def __init__(
module_name="ecalDigis", manipulator_name="EcalSrFlag", steps=["SIM", "MERGE"]
)
)
# this is needed by the HcalNoiseInfoProducer/'hcalnoise'
to_bemanipulate.append(
module_manipulate(
module_name="hcalDigis", manipulator_name="HcalDigi", steps=["SIM", "MERGE"]
)
)
to_bemanipulate.append(
module_manipulate(
module_name="electronMergedSeeds",
manipulator_name="ElectronSeed",
steps=["SIM", "MERGE"],
)
)
to_bemanipulate.append(
module_manipulate(
module_name="ecalDrivenElectronSeeds",
manipulator_name="EcalDrivenElectronSeed",
steps=["SIM", "MERGE"],
)
)

to_bemanipulate.append(
module_manipulate(
Expand All @@ -190,15 +167,9 @@ def __init__(
)
)

to_bemanipulate.append(
module_manipulate(module_name="hbheprereco", manipulator_name="HBHERecHit")
winterchristian marked this conversation as resolved.
Show resolved Hide resolved
)
to_bemanipulate.append(
module_manipulate(module_name="hbhereco", manipulator_name="HBHERecHit")
)
to_bemanipulate.append(
module_manipulate(module_name="zdcreco", manipulator_name="ZDCRecHit")
)

to_bemanipulate.append(
module_manipulate(module_name="horeco", manipulator_name="HORecHit")
Expand Down Expand Up @@ -583,8 +554,6 @@ def customiseGenerator_preHLT(process, changeProcessname=True, reselect=False):
)

# Disable noise simulation
process.mix.digitizers.castor.doNoise = cms.bool(False)

process.mix.digitizers.ecal.doESNoise = cms.bool(False)
process.mix.digitizers.ecal.doENoise = cms.bool(False)

Expand Down Expand Up @@ -639,6 +608,19 @@ def customiseGenerator_HLT(process, changeProcessname=True, reselect=False):
process.embeddingHltPixelVertices.clone()
)

# replace the original detector state filters in the HLT with a dummy module
process.hltPixelTrackerHVOn = cms.EDFilter("EmbeddingDetectorStateFilter",
Copy link
Contributor

@mmusich mmusich Feb 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I have to admit I am having difficulties to understand the logic here (@cms-sw/hlt-l2 FYI).
If you want to replace these two filters with something that always pass, I guess you can just use this configuration:

process.hltPixelTrackerHVOn = cms.EDFilter("HLTBool",
    result = cms.bool(True)
)

without the need of creating a new module EmbeddingDetectorStateFilter that does nothing.

But more in general can you explain what goes wrong with the regular module we use at HLT?
In the embedded event from where do the unpacked online metadata digis come from? From the real data event or the embedded MC event?

For the record, in the current menu we configure these modules as:

fragment.hltPixelTrackerHVOn = cms.EDFilter( "DetectorStateFilter",
    DebugOn = cms.untracked.bool( False ),
    DetectorType = cms.untracked.string( "pixel" ),
    acceptedCombinations = cms.untracked.vstring(  ),
    DcsStatusLabel = cms.untracked.InputTag( "" ),
    DCSRecordLabel = cms.untracked.InputTag( "hltOnlineMetaDataDigis" )
)

fragment.hltStripTrackerHVOn = cms.EDFilter( "DetectorStateFilter",
    DebugOn = cms.untracked.bool( False ),
    DetectorType = cms.untracked.string( "sistrip" ),
    acceptedCombinations = cms.untracked.vstring(  ),
    DcsStatusLabel = cms.untracked.InputTag( "" ),
    DCSRecordLabel = cms.untracked.InputTag( "hltOnlineMetaDataDigis" )
)

Copy link

Choose a reason for hiding this comment

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

Hi, thanks for giving us this hint. We are going to change the implementation according to your proposal! The reason why we introduced a module which always evaluates to True is an issue with the DetectorStateFilter when processing embedded events. The two filters reject 100% of our events. (The results of corresponding study can be found in this presentation: https://indico.cern.ch/event/1389181/contributions/5841911/attachments/2817132/4918642/2024-03-11-tau-cqm-meeting-triggers-in-mu-to-tau-embedded-events.pdf.) To our understanding, this is related to the fact that embedded events are "real data" events, but the detector in the HLT simulation is a simulated detector. Thus, the module always tries to read out the detector status of the real detector, while a simulated detector is present in our HLT simulation of the embedded event. With our implementation, we want to emulate the behavior of this module on MC events, which always pass this filter.

Copy link
Contributor

Choose a reason for hiding this comment

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

@moritzmolch is there a cmsDriver that can be used to reproduce such behavior?

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure what you want to achieve with a cmsDriver command.
First of all the tau embedding datasets are completely separated from normal cms or MC datasets.
You can find those datasets for Run2 UL here as described in this twiki.
In those some HLT filters perform worse or have an efficiency of 0%.

This is because the tau embedding method simulates two tau decays in an otherwise empty detector. If the HLT step now runs on these events, it makes perfect sense that some filters that expect a busy background will not work properly in this empty detector with only the tau decays.

This is therefore expected behavior up to a certain point. We solve this problem by switching off these problematic filters or changing them so that they allow all events.

Sorry if this was not clear and feel free to ask if you want to know more about this.
I also plan to give a presentation about the tau embedding method in the next RECO meeting.

Copy link
Contributor

@mmusich mmusich Feb 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

If the HLT step now runs on these events, it makes perfect sense that some filters that expect a busy background will not work properly in this empty detector with only the tau decays.

OK, but in this PR we're discussing specifically these two filters: hltPixelTrackerHVOn and hltStripTrackerHVOn.
These filters don't care about event occupancy. That you have 10k tracks or none at all, it doesn't matter.
What matters is the aggregate DCS state of the detector which can be fully ON, partially ON or fully OFF.
Now, in MC events the filter is designed to accept all the events, see

cmssw/DQM/TrackerCommon/plugins/DetectorStateFilter.cc

Lines 308 to 314 in 2e0d63c

} else {
detectorOn_ = true;
nSelectedEvents_++;
if (verbose_) {
edm::LogInfo("DetectorStatusFilter") << "Total MC Events " << nEvents_ << " Selected Events " << nSelectedEvents_
<< " Detector States " << detectorOn_ << std::endl;
}

so you wouldn't need to bypass them.
In real data, it depends on what was the actual state of the detector in that particular event, but assuming you are able to reconstruct taus in it, it looks to me very unlikely that any of the tracker partitions was OFF.
So all in all, I am sorry, but this doesn't make any sense to me.

Now coming back to this:

I'm not sure what you want to achieve with a cmsDriver command.

what I want to achieve is to see for myself the answer to the question I asked at #47299 (comment)

In the embedded event from where do the unpacked online metadata digis come from? From the real data event or the embedded MC event?

as I haven't received any answer.

Copy link
Author

@winterchristian winterchristian Feb 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Now, in MC events the filter is designed to accept all the events, ... In real data, it depends on what was the actual state of the detector in that particular event, but assuming you are able to reconstruct taus in it, it looks to me very unlikely that any of the tracker partitions was OFF. So all in all, I am sorry, but this doesn't make any sense to me.

Ah, now this makes sense. Thanks for pointing that out. While looking at the code you linked, I realized that this is decided by evt.isRealData(), which calls a function in EventAuxiliary. I looked into our embedding files and there isRealData is always True. So that could be the reason this trigger performs not as expected.

what I want to achieve is to see for myself the answer to the question I asked at #47299 (comment)

Sorry for this, I'm not an expert on triggers and didn't understand your goal with this.
I will try to answer at my understanding:

But more in general can you explain what goes wrong with the regular module we use at HLT?

I guess it is connected to the evt.isRealData() as described above.

In the embedded event from where do the unpacked online metadata digis come from? From the real data event or the embedded MC event?

Can you please tell me what are unpacked online metadata digis?

And for the cmsDriver commands: I have described how to produce embedding samples in my presentation at a TAU POG meeting last week. You can find the slides here and on slide 25 the cmsDriver command for the HLT step. As input file for the HLT step you can use root://xrootd-cms.infn.it//store/user/cwinter/run3_embedding/2022postEE/MuMu/gensim/0_gensim.root.

And also thanks a lot for your effort to get to the ground of our problems here, I'm very grateful of your help with all this.

DcsStatusLabel = cms.untracked.InputTag("hltScalersRawToDigi"),
DebugOn = cms.untracked.bool(False),
DetectorType = cms.untracked.string('pixel')
)

process.hltStripTrackerHVOn = cms.EDFilter("EmbeddingDetectorStateFilter",
DcsStatusLabel = cms.untracked.InputTag("hltScalersRawToDigi"),
DebugOn = cms.untracked.bool(False),
DetectorType = cms.untracked.string('sistrip')
)

process = customisoptions(process)
##process = fix_input_tags(process)

Expand Down Expand Up @@ -801,12 +783,10 @@ def customiseMerging(process, changeProcessname=True, reselect=False):

print("**** Attention: overriding behaviour of 'removeMCMatching' ****")

import PhysicsTools.PatAlgos.tools.coreTools
from PhysicsTools.PatAlgos.slimming.miniAOD_tools import miniAOD_customizeMC
def performMCMatching(process, names, postfix, outputModules):
miniAOD_customizeMC(process)

import PhysicsTools.PatAlgos.tools.coreTools
PhysicsTools.PatAlgos.tools.coreTools.removeMCMatching = performMCMatching
PhysicsTools.PatAlgos.tools.coreTools.removeMCMatching = lambda process, names, postfix, outputModules : miniAOD_customizeMC(process)

if changeProcessname:
process._Process__name = "MERGE"
Expand Down Expand Up @@ -893,42 +873,10 @@ def performMCMatching(process, names, postfix, outputModules):
)
process.dedxHitInfo.clusterShapeCache = cms.InputTag("")

# process.muons.FillDetectorBasedIsolation = cms.bool(False)
# process.muons.FillSelectorMaps = cms.bool(False)
# process.muons.FillShoweringInfo = cms.bool(False)
# process.muons.FillCosmicsIdMap = cms.bool(False)

# process.displacedMuons.FillDetectorBasedIsolation = cms.bool(False)
# process.displacedMuons.FillSelectorMaps = cms.bool(False)
# process.displacedMuons.FillShoweringInfo = cms.bool(False)
# process.displacedMuons.FillCosmicsIdMap = cms.bool(False)

# seed configuration needed for seedmerger
#process.load(
# "RecoEgamma.EgammaElectronProducers.ecalDrivenElectronSeedsParameters_cff"
#)
#process.ecalDrivenElectronSeeds.SeedConfiguration = cms.PSet(
# process.ecalDrivenElectronSeedsParameters
#)

process.merge_step += process.highlevelreco
# process.merge_step.remove(process.reducedEcalRecHitsEE)
# process.merge_step.remove(process.reducedEcalRecHitsEB)

# process.merge_step.remove(process.ak4JetTracksAssociatorExplicit)

# process.merge_step.remove(process.cosmicsVeto)
# process.merge_step.remove(process.cosmicsVetoTrackCandidates)
# process.merge_step.remove(process.ecalDrivenGsfElectronCores)
# process.merge_step.remove(process.ecalDrivenGsfElectrons)
# process.merge_step.remove(process.gedPhotonsTmp)
# process.merge_step.remove(process.particleFlowTmp)

# process.merge_step.remove(process.hcalnoise)

process.load("CommonTools.ParticleFlow.genForPF2PAT_cff")

# process.muonsFromCosmics.ShowerDigiFillerParameters.dtDigiCollectionLabel = cms.InputTag("simMuonDTDigis")

process.merge_step += process.genForPF2PATSequence

Expand Down Expand Up @@ -970,6 +918,7 @@ def performMCMatching(process, names, postfix, outputModules):
if name in reconstruction_modules_list:
modules_to_be_ordered[name] = reconstruction_modules_list.index(name)
else:
import sys
print("ERROR:",name,"not prepared in modules list. Please adapt 'customiseMerging'")
sys.exit(1)

Expand Down