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

Jump to bottom

Associated PF for Egamma in 2018 PbPb miniAOD #37262

Merged
merged 5 commits into from
Apr 25, 2022
Merged

Associated PF for Egamma in 2018 PbPb miniAOD #37262

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
be8d658
Fix GED electron/photon PF association in run2 heavy-ion miniAOD
ttrk Mar 17, 2022
68ac690
apply code format
ttrk Mar 17, 2022
9246910
Apply suggestions from code review
ttrk Mar 29, 2022
f9b4aef
fix type
ttrk Mar 30, 2022
a9fc49b
Implement review comments https://github.com/cms-sw/cmssw/pull/37262#…
ttrk Apr 16, 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
1 change: 1 addition & 0 deletions PhysicsTools/PatAlgos/python/slimming/slimming_cff.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@
from Configuration.ProcessModifiers.run2_miniAOD_pp_on_AA_103X_cff import run2_miniAOD_pp_on_AA_103X
run2_miniAOD_pp_on_AA_103X.toReplaceWith(slimmingTask,cms.Task(primaryVertexAssociationCleaned,slimmingTask.copy()))
run2_miniAOD_pp_on_AA_103X.toReplaceWith(slimmingTask,cms.Task(primaryVertexWithBSAssociationCleaned,slimmingTask.copy()))
run2_miniAOD_pp_on_AA_103X.toReplaceWith(slimmingTask,cms.Task(pfEGammaToCandidateRemapperCleaned,slimmingTask.copy()))

from RecoHI.HiTracking.miniAODVertexRecovery_cff import offlinePrimaryVerticesRecovery, offlineSlimmedPrimaryVerticesRecovery
pp_on_AA.toReplaceWith(
Expand Down
11 changes: 11 additions & 0 deletions RecoEgamma/EgammaPhotonProducers/python/reducedEgamma_cfi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,3 +104,14 @@
hiPhotonIsolationMapInput = "photonIsolationHIProducerppGED",
hiPhotonIsolationMapOutput = "photonIsolationHIProducerppGED"
)

from RecoHI.HiJetAlgos.HiBadParticleCleaner_cfi import cleanedParticleFlow
from CommonTools.ParticleFlow.pfEGammaToCandidateRemapper_cfi import pfEGammaToCandidateRemapper as pfEGammaToCandidateRemapperCleaned
pfEGammaToCandidateRemapperCleaned.pf2pf = cms.InputTag("cleanedParticleFlow")
Copy link
Contributor

Choose a reason for hiding this comment

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

Please clone pfEGammaToCandidateRemapperCleaned from pfEGammaToCandidateRemapper inside the pfEGammaToCandidateRemapper_cfi.py: modules should be modified in the very same configuration file in which they are defined, not in random places elsewhere in cmssw.
(You can follow as example how the primaryVertexAssociationCleaned is cloned and modifiel inside primaryVertexAssociation_cfi.py)


from Configuration.ProcessModifiers.run2_miniAOD_pp_on_AA_103X_cff import run2_miniAOD_pp_on_AA_103X
run2_miniAOD_pp_on_AA_103X.toModify(
reducedEgamma,
photonsPFValMap = cms.InputTag("pfEGammaToCandidateRemapperCleaned:photons"),
gsfElectronsPFValMap = cms.InputTag("pfEGammaToCandidateRemapperCleaned:electrons")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
photonsPFValMap = cms.InputTag("pfEGammaToCandidateRemapperCleaned:photons"),
gsfElectronsPFValMap = cms.InputTag("pfEGammaToCandidateRemapperCleaned:electrons")
photonsPFValMap = "pfEGammaToCandidateRemapperCleaned:photons",
gsfElectronsPFValMap = "pfEGammaToCandidateRemapperCleaned:electrons"

)
45 changes: 40 additions & 5 deletions RecoHI/HiJetAlgos/plugins/HiBadParticleCleaner.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class HiBadParticleCleaner : public edm::global::EDProducer<> {
void produce(edm::StreamID, edm::Event&, const edm::EventSetup&) const override;
// ----------member data ---------------------------

edm::EDGetTokenT<edm::View<reco::PFCandidate> > tokenPFCandidates_;
edm::EDGetTokenT<edm::View<reco::PFCandidate>> tokenPFCandidates_;
edm::EDGetTokenT<reco::VertexCollection> tokenPV_;

const double minMuonPt_;
Expand All @@ -51,7 +51,7 @@ class HiBadParticleCleaner : public edm::global::EDProducer<> {
// constructors and destructor
//
HiBadParticleCleaner::HiBadParticleCleaner(const edm::ParameterSet& iConfig)
: tokenPFCandidates_(consumes<edm::View<reco::PFCandidate> >(iConfig.getParameter<edm::InputTag>("PFCandidates"))),
: tokenPFCandidates_(consumes<edm::View<reco::PFCandidate>>(iConfig.getParameter<edm::InputTag>("PFCandidates"))),
tokenPV_(consumes<reco::VertexCollection>(iConfig.getParameter<edm::InputTag>("offlinePV"))),
minMuonPt_(iConfig.getParameter<double>("minMuonPt")),
minChargedHadronPt_(iConfig.getParameter<double>("minChargedHadronPt")),
Expand All @@ -66,6 +66,7 @@ HiBadParticleCleaner::HiBadParticleCleaner(const edm::ParameterSet& iConfig)
produces<bool>();
produces<reco::PFCandidateCollection>();
produces<reco::PFCandidateCollection>("removed");
produces<edm::ValueMap<reco::PFCandidateRef>>();
}

//
Expand All @@ -91,7 +92,11 @@ void HiBadParticleCleaner::produce(edm::StreamID, edm::Event& iEvent, const edm:

bool foundBadCandidate = false;

size_t n = pfCandidates->size();
std::vector<int> oldToNew(n);
Copy link
Contributor

Choose a reason for hiding this comment

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

This oldToNew map is something intended as a temporary solution, waiting for a fix on this

This is understood as packedPFcandidates in this workflow is made from an intermediate, "cleaned" PF collection (introduced via #31668) and while references in particleBasedIsolation are still the ones from original PF collection.

?

Or as definitive fix?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was intended as definitive fix for that module.

Copy link
Contributor

@mandrenguyen mandrenguyen Mar 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

It's temporary in the sense that it's only needed for reMiniAOD of the existing 2018 PbPb AOD. If we were to re-reconstruct the 2018 data, we wouldn't need this. We will also not need it for Run 3 (assuming we don't screw up the track quality selections again).

Copy link
Contributor

Choose a reason for hiding this comment

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

If this has to be a definitive fix I am convinced that a more meaningfull name than oldToNew could be invented.
Since the size is known, it could also probably more conveniently became a std::array<int, n>

size_t iPF = -1;
Copy link
Contributor

Choose a reason for hiding this comment

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

size_t is an unsigned integral type: how can you initialize it to "-1"?
Probably iPF++ does the right job nonetheless here below, but it is at least confusing using an unsigned type as such (and I think a few compilers could also complain)

for (const reco::PFCandidate& pfCandidate : *pfCandidates) {
iPF++;
if (pfCandidate.particleId() == reco::PFCandidate::ParticleType::mu) // muon cleaning
{
if (pfCandidate.pt() > minMuonPt_) {
Expand Down Expand Up @@ -128,19 +133,22 @@ void HiBadParticleCleaner::produce(edm::StreamID, edm::Event& iEvent, const edm:

if (sig3d > maxSigLoose_) {
pBadCandidateCollection->push_back(pfCandidate);
oldToNew[iPF] = -1 * (pBadCandidateCollection->size());
foundBadCandidate = true;
continue;
}

if (track->pt() < pfCandidate.pt() / 1.5 || track->pt() > pfCandidate.pt() * 1.5) {
foundBadCandidate = true;
pBadCandidateCollection->push_back(pfCandidate);
oldToNew[iPF] = -1 * (pBadCandidateCollection->size());
continue;
}
if (track->originalAlgo() == reco::TrackBase::muonSeededStepOutIn &&
track->hitPattern().trackerLayersWithMeasurement() < minTrackerLayersForMuonTight_) {
foundBadCandidate = true;
pBadCandidateCollection->push_back(pfCandidate);
oldToNew[iPF] = -1 * (pBadCandidateCollection->size());
continue;
}
}
Expand All @@ -156,6 +164,7 @@ void HiBadParticleCleaner::produce(edm::StreamID, edm::Event& iEvent, const edm:
if ((nHits < minTrackNHits_ && nPixelHits < minPixelNHits_) || nHits == 3) {
foundBadCandidate = true;
pBadCandidateCollection->push_back(pfCandidate);
oldToNew[iPF] = -1 * (pBadCandidateCollection->size());
continue;
}

Expand All @@ -176,12 +185,14 @@ void HiBadParticleCleaner::produce(edm::StreamID, edm::Event& iEvent, const edm:
if (sig3d > maxSigLoose_) {
foundBadCandidate = true;
pBadCandidateCollection->push_back(pfCandidate);
oldToNew[iPF] = -1 * (pBadCandidateCollection->size());
continue;
}

if (sig3d > maxSigTight_ && nHits < minTrackNHits_) {
foundBadCandidate = true;
pBadCandidateCollection->push_back(pfCandidate);
oldToNew[iPF] = -1 * (pBadCandidateCollection->size());
continue;
}

Expand All @@ -192,12 +203,14 @@ void HiBadParticleCleaner::produce(edm::StreamID, edm::Event& iEvent, const edm:
if (sig3d > maxSigLoose_) {
foundBadCandidate = true;
pBadCandidateCollection->push_back(pfCandidate);
oldToNew[iPF] = -1 * (pBadCandidateCollection->size());
continue;
}

if (nHits < minTrackNHits_) {
foundBadCandidate = true;
pBadCandidateCollection->push_back(pfCandidate);
oldToNew[iPF] = -1 * (pBadCandidateCollection->size());
continue;
}
}
Expand All @@ -208,34 +221,56 @@ void HiBadParticleCleaner::produce(edm::StreamID, edm::Event& iEvent, const edm:
if (sig3d > maxSigTight_) {
foundBadCandidate = true;
pBadCandidateCollection->push_back(pfCandidate);
oldToNew[iPF] = -1 * (pBadCandidateCollection->size());
continue;
}

if (nHits < minTrackNHits_) {
foundBadCandidate = true;
pBadCandidateCollection->push_back(pfCandidate);
oldToNew[iPF] = -1 * (pBadCandidateCollection->size());
continue;
}

if (nPixelHits < minPixelNHits_) {
foundBadCandidate = true;
pBadCandidateCollection->push_back(pfCandidate);
oldToNew[iPF] = -1 * (pBadCandidateCollection->size());
continue;
}
}
}
}

pOutputCandidateCollection->push_back(pfCandidate);

oldToNew[iPF] = (pOutputCandidateCollection->size());
} // end loop over pf candidates

bool pass = !foundBadCandidate;

iEvent.put(std::move(pOutputCandidateCollection));
iEvent.put(std::move(pBadCandidateCollection), "removed");
edm::OrphanHandle<std::vector<reco::PFCandidate>> newpf = iEvent.put(std::move(pOutputCandidateCollection));
edm::OrphanHandle<std::vector<reco::PFCandidate>> badpf = iEvent.put(std::move(pBadCandidateCollection), "removed");

iEvent.put(std::make_unique<bool>(pass));

std::unique_ptr<edm::ValueMap<reco::PFCandidateRef>> pf2pf(new edm::ValueMap<reco::PFCandidateRef>());
edm::ValueMap<reco::PFCandidateRef>::Filler filler(*pf2pf);

std::vector<reco::PFCandidateRef> refs;
refs.reserve(n);

// old to new
for (iPF = 0; iPF < n; ++iPF) {
if (oldToNew[iPF] > 0) {
refs.push_back(reco::PFCandidateRef(newpf, oldToNew[iPF] - 1));
} else {
refs.push_back(reco::PFCandidateRef(badpf, -oldToNew[iPF] - 1));
}
}
filler.insert(pfCandidates, refs.begin(), refs.end());

filler.fill();
iEvent.put(std::move(pf2pf));
}

//define this as a plug-in
Expand Down