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

Adding vertex recovery step for 2018 heavy ion miniAOD production. #31676

Merged
merged 10 commits into from
Oct 29, 2020
Merged

Adding vertex recovery step for 2018 heavy ion miniAOD production. #31676

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a0881f5
added Vertex Recovery option for HI
abaty Oct 2, 2020
9f7ab42
added code format and checks
abaty Oct 6, 2020
21e5a2b
made import more specific to avoid bugs and included default param in…
abaty Oct 7, 2020
f473f65
fixed conflict from recent merges
abaty Oct 10, 2020
f55dc9b
Merge branch 'master' into miniAOD_VtxRecovery
abaty Oct 10, 2020
0b5056b
added assert and tweaked configs for vertex recovery
abaty Oct 23, 2020
ee65572
Merge branch 'miniAOD_VtxRecovery' of https://github.com/abaty/cmssw …
abaty Oct 23, 2020
280b0c5
fixes for config error and switch to exception error message
abaty Oct 27, 2020
d552e8b
small change to pass code checks
abaty Oct 27, 2020
7d0d84c
Merge branch 'master' into miniAOD_VtxRecovery
abaty Oct 27, 2020
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/MicroEventContent_cff.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@
'keep recoCentrality_hiCentrality_*_*',
'keep int_centralityBin_*_*',
'keep recoHFFilterInfo_hiHFfilters_*_*',
'keep *_offlineSlimmedPrimaryVerticesRecovery_*_*',
]
from Configuration.Eras.Modifier_pp_on_AA_2018_cff import pp_on_AA_2018
from Configuration.Eras.Modifier_pp_on_PbPb_run3_cff import pp_on_PbPb_run3
Expand Down
5 changes: 5 additions & 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 @@ -79,6 +79,11 @@
slimmingTask,
cms.Task(slimmingTask.copy(), packedCandidateMuonID, packedPFCandidateTrackChi2, lostTrackChi2, centralityBin, hiHFfilters))

from RecoHI.HiTracking.miniAODVertexRecovery_cff import offlinePrimaryVerticesRecovery, offlineSlimmedPrimaryVerticesRecovery
pp_on_AA_2018.toReplaceWith(
slimmingTask,
cms.Task(slimmingTask.copy(), offlinePrimaryVerticesRecovery, offlineSlimmedPrimaryVerticesRecovery))

from Configuration.Eras.Modifier_phase2_timing_cff import phase2_timing
_phase2_timing_slimmingTask = cms.Task(slimmingTask.copy(),
offlineSlimmedPrimaryVertices4D)
Expand Down
49 changes: 49 additions & 0 deletions RecoHI/HiTracking/python/miniAODVertexRecovery_cff.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import FWCore.ParameterSet.Config as cms

from RecoVertex.PrimaryVertexProducer.OfflinePrimaryVertices_cfi import offlinePrimaryVertices

offlinePrimaryVerticesRecovery = offlinePrimaryVertices.clone(

isRecoveryIteration = cms.bool(True),
recoveryVtxCollection = cms.InputTag("offlinePrimaryVertices"),

TkFilterParameters = cms.PSet(
algorithm=cms.string('filter'),
maxNormalizedChi2 = cms.double(999.0),
minPixelLayersWithHits=cms.int32(0),
minSiliconLayersWithHits = cms.int32(0),
maxD0Significance = cms.double(999.0),
minPt = cms.double(0.0),
maxEta = cms.double(999.0),
trackQuality = cms.string("any")
),

TkClusParameters = cms.PSet(
algorithm = cms.string("gap"),
TkGapClusParameters = cms.PSet(
zSeparation = cms.double(1.0)
)
),

vertexCollections = cms.VPSet(
[cms.PSet(label=cms.string(""),
algorithm=cms.string("AdaptiveVertexFitter"),
chi2cutoff = cms.double(2.5),
minNdof=cms.double(0.0),
useBeamConstraint = cms.bool(False),
maxDistanceToBeam = cms.double(1.0)
),
]
)
Copy link
Contributor

Choose a reason for hiding this comment

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

please drop type specifications for all parameters which already exist.
This is a safer way to protect from parameter name mistakes and will also help in possible parameter migrations.
Also, replace only parameters that were actually changed.

Suggested change
offlinePrimaryVerticesRecovery = offlinePrimaryVertices.clone(
isRecoveryIteration = cms.bool(True),
recoveryVtxCollection = cms.InputTag("offlinePrimaryVertices"),
TkFilterParameters = cms.PSet(
algorithm=cms.string('filter'),
maxNormalizedChi2 = cms.double(999.0),
minPixelLayersWithHits=cms.int32(0),
minSiliconLayersWithHits = cms.int32(0),
maxD0Significance = cms.double(999.0),
minPt = cms.double(0.0),
maxEta = cms.double(999.0),
trackQuality = cms.string("any")
),
TkClusParameters = cms.PSet(
algorithm = cms.string("gap"),
TkGapClusParameters = cms.PSet(
zSeparation = cms.double(1.0)
)
),
vertexCollections = cms.VPSet(
[cms.PSet(label=cms.string(""),
algorithm=cms.string("AdaptiveVertexFitter"),
chi2cutoff = cms.double(2.5),
minNdof=cms.double(0.0),
useBeamConstraint = cms.bool(False),
maxDistanceToBeam = cms.double(1.0)
),
]
)
offlinePrimaryVerticesRecovery = offlinePrimaryVertices.clone(
isRecoveryIteration = True,
recoveryVtxCollection = "offlinePrimaryVertices",
TkFilterParameters = dict(
maxNormalizedChi2 = 999.0,
minPixelLayersWithHits = 0,
minSiliconLayersWithHits = 0,
maxD0Significance = 999.0,
maxEta = 999.0,
trackQuality = "any"
),
vertexCollections = [offlinePrimaryVertices.vertexCollections[0].clone()]
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@slava77 The default offlinePrimaryVertices has a default minPt of 0, but is modified by the pp_on_AA_2018 era to have minPt = 0.7. When the clone is done, is the default value (0) taken, or does the era-modified version get taken for the new module (in which case we need to specify minPt = 0 again).

Copy link
Contributor

Choose a reason for hiding this comment

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

@slava77 The default offlinePrimaryVertices has a default minPt of 0, but is modified by the pp_on_AA_2018 era to have minPt = 0.7. When the clone is done, is the default value (0) taken, or does the era-modified version get taken for the new module (in which case we need to specify minPt = 0 again).

I expect that the modified version after pp_on_AA_2018 is what you will see here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the info, this is also what I expected. I will need to include a few extra parameters that are not listed in the example here.


)

from PhysicsTools.PatAlgos.slimming.offlineSlimmedPrimaryVertices_cfi import offlineSlimmedPrimaryVertices
offlineSlimmedPrimaryVerticesRecovery = cms.EDProducer("PATVertexSlimmer",
src = cms.InputTag("offlinePrimaryVerticesRecovery"),
)

#offlineSlimmedPrimaryVerticesRecovery = offlineSlimmedPrimaryVertices.clone(
# src = cms.InputTag("offlinePrimaryVerticesRecovery"),
# score = cms.InputTag(""),
#)
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the reason for not cloning with a replace?
Please either cleanup the commented-out alternative (including the import of the original) or use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When using the clone implementation, 'score' is still defined which causes issues with a existsAs() call in the PATVertexSlimmer to look for a collection that is not created (and not needed for our use). Thus, I have remove the commented section and gone with the first implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

You can remove parameters in clone()/toModify() with None

offlineSlimmedPrimaryVerticesRecovery = offlineSlimmedPrimaryVertices.clone(
    src = "offlinePrimaryVerticesRecovery",
    score = None
)

3 changes: 3 additions & 0 deletions RecoVertex/PrimaryVertexProducer/interface/PrimaryVertexProducer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ class PrimaryVertexProducer : public edm::stream::EDProducer<> {
edm::ParameterSet theConfig;
bool fVerbose;

bool isRecoveryIteration;
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
bool isRecoveryIteration;
bool fRecoveryIteration;

to follow the style of naming of other flags in this module

edm::EDGetTokenT<reco::VertexCollection> recoveryVtxToken;

edm::EDGetTokenT<reco::BeamSpot> bsToken;
edm::EDGetTokenT<reco::TrackCollection> trkToken;
edm::EDGetTokenT<edm::ValueMap<float> > trkTimesToken;
Expand Down
28 changes: 28 additions & 0 deletions RecoVertex/PrimaryVertexProducer/plugins/PrimaryVertexProducer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ PrimaryVertexProducer::PrimaryVertexProducer(const edm::ParameterSet& conf) : th
bsToken = consumes<reco::BeamSpot>(conf.getParameter<edm::InputTag>("beamSpotLabel"));
f4D = false;

//check if this is a recovery iteration
isRecoveryIteration = conf.getParameter<bool>("isRecoveryIteration");
if (isRecoveryIteration) {
recoveryVtxToken = consumes<reco::VertexCollection>(conf.getParameter<edm::InputTag>("recoveryVtxCollection"));
}

// select and configure the track selection
std::string trackSelectionAlgorithm =
conf.getParameter<edm::ParameterSet>("TkFilterParameters").getParameter<std::string>("algorithm");
Expand Down Expand Up @@ -149,6 +155,26 @@ void PrimaryVertexProducer::produce(edm::Event& iEvent, const edm::EventSetup& i
edm::LogError("UnusableBeamSpot") << "Beamspot with invalid errors " << beamVertexState.error().matrix();
}

//if this is a recovery iteration, check if we already have a valid PV
if (isRecoveryIteration) {
edm::Handle<reco::VertexCollection> oldPVs;
iEvent.getByToken(recoveryVtxToken, oldPVs);
const reco::VertexCollection* oldVertices;
oldVertices = oldPVs.product();
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
edm::Handle<reco::VertexCollection> oldPVs;
iEvent.getByToken(recoveryVtxToken, oldPVs);
const reco::VertexCollection* oldVertices;
oldVertices = oldPVs.product();
auto const& oldVertices = iEvent.get(recoveryVtxToken);

more modern less verbose syntax

//look for the first valid (not-BeamSpot) vertex
for (size_t i = 0; i < oldVertices->size(); ++i) {
if (!((*oldVertices)[i].isFake())) {
//found a valid vertex, write the first one to the collection and return
//otherwise continue with regular vertexing procedure
auto result = std::make_unique<reco::VertexCollection>();
reco::VertexCollection& vColl = (*result);
vColl.push_back((*oldVertices)[i]);
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
for (size_t i = 0; i < oldVertices->size(); ++i) {
if (!((*oldVertices)[i].isFake())) {
//found a valid vertex, write the first one to the collection and return
//otherwise continue with regular vertexing procedure
auto result = std::make_unique<reco::VertexCollection>();
reco::VertexCollection& vColl = (*result);
vColl.push_back((*oldVertices)[i]);
for (auto const& old : oldVertices) {
if (!(old.isFake())) {
//found a valid vertex, write the first one to the collection and return
//otherwise continue with regular vertexing procedure
auto result = std::make_unique<reco::VertexCollection>();
result->push_back(old);

iEvent.put(std::move(result), algorithms.begin()->label);
Copy link
Contributor

Choose a reason for hiding this comment

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

the same should be done for all available algorithms or there should be an assert in the constructor that only one algorithm exists if isRecoveryIteration is set

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added an assert() in the constructor. For our purposes of event selection, we only need one algorithm run for the recovery.

return;
}
}
}

// get RECO tracks from the event
// `tks` can be used as a ptr to a reco::TrackCollection
edm::Handle<reco::TrackCollection> tks;
Expand Down Expand Up @@ -414,6 +440,8 @@ void PrimaryVertexProducer::fillDescriptions(edm::ConfigurationDescriptions& des
psd0.add<std::string>("algorithm", "DA_vect");
desc.add<edm::ParameterSetDescription>("TkClusParameters", psd0);
}
desc.add<bool>("isRecoveryIteration", false);
desc.add<edm::InputTag>("recoveryVtxCollection", edm::InputTag("offlinePrimaryVertices"));
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
desc.add<edm::InputTag>("recoveryVtxCollection", edm::InputTag("offlinePrimaryVertices"));
desc.add<edm::InputTag>("recoveryVtxCollection", {""});

to match what's actually added in RecoVertex/PrimaryVertexProducer/python/OfflinePrimaryVertices_cfi.py


descriptions.add("primaryVertexProducer", desc);
}
Expand Down
6 changes: 4 additions & 2 deletions RecoVertex/PrimaryVertexProducer/python/OfflinePrimaryVertices_cfi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,10 @@
maxDistanceToBeam = cms.double(1.0),
)
]
)

),

isRecoveryIteration = cms.bool(False),
recoveryVtxCollection = cms.InputTag("")


)
Expand Down