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

[PF] Add fillDescription to RecoParticleFlow producers #45212

Merged
merged 7 commits into from
Jun 21, 2024
Merged

[PF] Add fillDescription to RecoParticleFlow producers #45212

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
41f08d0
Add fillDescription to RecoParticleFlow producers
Jun 13, 2024
7b34707
Fixed issue and implemented comments
Jun 13, 2024
50d3e58
Remove exists and fix fill descriptions
Jun 15, 2024
2da72f0
Remove the unknown PF parameter settings
Jun 16, 2024
a848a6c
Implemented comments
Jun 16, 2024
c85415b
Remove optional condition on parameters
Jun 18, 2024
9fee0bb
Undo the renaming of gathering, seeding and reHitEnergyNorm double ve…
Jun 19, 2024
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
18 changes: 14 additions & 4 deletions RecoParticleFlow/Configuration/plugins/HepMCCopy.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
#include "FWCore/Framework/interface/one/EDProducer.h"
#include "FWCore/ParameterSet/interface/ParameterSet.h"
#include "FWCore/ParameterSet/interface/ConfigurationDescriptions.h"
#include "FWCore/Framework/interface/Event.h"
#include "FWCore/ServiceRegistry/interface/Service.h"
#include "SimDataFormats/GeneratorProducts/interface/HepMCProduct.h"
Expand All @@ -10,19 +12,27 @@ class HepMCCopy : public edm::one::EDProducer<> {
explicit HepMCCopy(edm::ParameterSet const& p);
~HepMCCopy() override = default;
void produce(edm::Event& e, const edm::EventSetup& c) override;
static void fillDescriptions(edm::ConfigurationDescriptions& descriptions);

private:
const edm::EDGetTokenT<edm::HepMCProduct> hepMCToken_;
};

HepMCCopy::HepMCCopy(edm::ParameterSet const& p) {
void HepMCCopy::fillDescriptions(edm::ConfigurationDescriptions& descriptions) {
edm::ParameterSetDescription desc;
desc.add<edm::InputTag>("src", {"generatorSmeared"});
descriptions.addWithDefaultLabel(desc);
}

HepMCCopy::HepMCCopy(edm::ParameterSet const& p)
: hepMCToken_(consumes<edm::HepMCProduct>(p.getParameter<edm::InputTag>("src"))) {
// This producer produces a HepMCProduct, a copy of the original one
produces<edm::HepMCProduct>();
}

void HepMCCopy::produce(edm::Event& iEvent, const edm::EventSetup& es) {
edm::Handle<edm::HepMCProduct> theHepMCProduct;
bool source = iEvent.getByLabel("generatorSmeared", theHepMCProduct);
if (!source) {
const auto& theHepMCProduct = iEvent.getHandle(hepMCToken_);
if (theHepMCProduct.isValid()) {
auto pu_product = std::make_unique<edm::HepMCProduct>();
iEvent.put(std::move(pu_product));
} else {
Expand Down
3 changes: 2 additions & 1 deletion RecoParticleFlow/Configuration/python/HepMCCopy_cfi.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import FWCore.ParameterSet.Config as cms

generator = cms.EDProducer("HepMCCopy")
from RecoParticleFlow.Configuration.HepMCCopy import HepMCCopy
generator = HepMCCopy()
stahlleiton marked this conversation as resolved.
Show resolved Hide resolved
10 changes: 8 additions & 2 deletions RecoParticleFlow/PFClusterProducer/plugins/PFClusterCollectionMerger.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,19 @@
class PFClusterCollectionMerger : public edm::global::EDProducer<> {
public:
PFClusterCollectionMerger(const edm::ParameterSet& conf) {
const std::vector<edm::InputTag>& inputs = conf.getParameter<std::vector<edm::InputTag> >("inputs");
const std::vector<edm::InputTag>& inputs = conf.getParameter<std::vector<edm::InputTag>>("inputs");
for (const auto& input : inputs) {
_inputs.push_back(consumes<reco::PFClusterCollection>(input));
}
produces<reco::PFClusterCollection>();
}

static void fillDescriptions(edm::ConfigurationDescriptions& descriptions) {
edm::ParameterSetDescription desc;
desc.add<std::vector<edm::InputTag>>("inputs", {});
descriptions.addWithDefaultLabel(desc);
}

void produce(edm::StreamID, edm::Event& e, const edm::EventSetup& es) const override {
auto output = std::make_unique<reco::PFClusterCollection>();
for (const auto& input : _inputs) {
Expand All @@ -34,7 +40,7 @@ class PFClusterCollectionMerger : public edm::global::EDProducer<> {
}

private:
std::vector<edm::EDGetTokenT<reco::PFClusterCollection> > _inputs;
std::vector<edm::EDGetTokenT<reco::PFClusterCollection>> _inputs;
};

DEFINE_FWK_MODULE(PFClusterCollectionMerger);
Expand Down
32 changes: 30 additions & 2 deletions RecoParticleFlow/PFClusterProducer/plugins/PFClusterProducer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class PFClusterProducer : public edm::stream::EDProducer<> {

void beginRun(const edm::Run&, const edm::EventSetup&) override;
void produce(edm::Event&, const edm::EventSetup&) override;
static void fillDescriptions(edm::ConfigurationDescriptions& descriptions);

private:
// inputs
Expand All @@ -33,8 +34,8 @@ class PFClusterProducer : public edm::stream::EDProducer<> {
// options
const bool _prodInitClusters;
// the actual algorithm
std::vector<std::unique_ptr<RecHitTopologicalCleanerBase> > _cleaners;
std::vector<std::unique_ptr<RecHitTopologicalCleanerBase> > _seedcleaners;
std::vector<std::unique_ptr<RecHitTopologicalCleanerBase>> _cleaners;
std::vector<std::unique_ptr<RecHitTopologicalCleanerBase>> _seedcleaners;
std::unique_ptr<SeedFinderBase> _seedFinder;
std::unique_ptr<InitialClusteringStepBase> _initialClustering;
std::unique_ptr<PFClusterBuilderBase> _pfClusterBuilder;
Expand All @@ -45,6 +46,33 @@ class PFClusterProducer : public edm::stream::EDProducer<> {
#include "FWCore/Framework/interface/MakerMacros.h"
DEFINE_FWK_MODULE(PFClusterProducer);

void PFClusterProducer::fillDescriptions(edm::ConfigurationDescriptions& descriptions) {
edm::ParameterSetDescription desc;
desc.add<edm::InputTag>("recHitsSource", {});
desc.add<bool>("usePFThresholdsFromDB", false);
edm::ParameterSetDescription psd;
psd.setUnknown();
stahlleiton marked this conversation as resolved.
Show resolved Hide resolved
desc.addVPSet("recHitCleaners", psd, {});
{
edm::ParameterSetDescription psd1;
psd1.add<std::vector<std::string>>("RecHitFlagsToBeExcluded", {});
psd1.add<std::string>("algoName");
desc.addVPSet("seedCleaners", psd1, {});
}
{
edm::ParameterSetDescription pset;
pset.add<std::string>("algoName");
pset.add<int>("nNeighbours", 0);
pset.addVPSet("thresholdsByDetector", psd, {});
desc.add<edm::ParameterSetDescription>("seedFinder", pset);
}
desc.add<edm::ParameterSetDescription>("initialClusteringStep", psd);
desc.add<edm::ParameterSetDescription>("pfClusterBuilder", psd);
desc.add<edm::ParameterSetDescription>("positionReCalc", psd);
desc.add<edm::ParameterSetDescription>("energyCorrector", psd);
descriptions.addWithDefaultLabel(desc);
}

#ifdef PFLOW_DEBUG
#define LOGVERB(x) edm::LogVerbatim(x)
#define LOGWARN(x) edm::LogWarning(x)
Expand Down
119 changes: 119 additions & 0 deletions RecoParticleFlow/PFClusterProducer/plugins/PFClusterTimeSelector.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ class PFClusterTimeSelector : public edm::stream::EDProducer<> {
void beginRun(const edm::Run& run, const edm::EventSetup& es) override;

void produce(edm::Event& iEvent, const edm::EventSetup& iSetup) override;
static void fillDescriptions(edm::ConfigurationDescriptions& descriptions);

protected:
struct CutInfo {
Expand All @@ -39,6 +40,124 @@ class PFClusterTimeSelector : public edm::stream::EDProducer<> {
#include "FWCore/Framework/interface/MakerMacros.h"
DEFINE_FWK_MODULE(PFClusterTimeSelector);

void PFClusterTimeSelector::fillDescriptions(edm::ConfigurationDescriptions& descriptions) {
edm::ParameterSetDescription desc;
desc.add<edm::InputTag>("src", {"particleFlowClusterECALWithTimeUncorrected"});
{
std::vector<edm::ParameterSet> vpset;
vpset.reserve(10);
{
edm::ParameterSet pset;
pset.addParameter<double>("depth", 1.0);
pset.addParameter<double>("minEnergy", 0.0);
pset.addParameter<double>("maxEnergy", 1.0);
pset.addParameter<bool>("endcap", false);
pset.addParameter<double>("minTime", -12.);
pset.addParameter<double>("maxTime", 12.);
vpset.emplace_back(pset);
}
{
edm::ParameterSet pset;
pset.addParameter<double>("depth", 1.0);
pset.addParameter<double>("minEnergy", 0.0);
pset.addParameter<double>("maxEnergy", 1.0);
pset.addParameter<bool>("endcap", true);
pset.addParameter<double>("minTime", -31.5);
pset.addParameter<double>("maxTime", 31.5);
vpset.emplace_back(pset);
}
{
edm::ParameterSet pset;
pset.addParameter<double>("depth", 1.0);
pset.addParameter<double>("minEnergy", 1.0);
pset.addParameter<double>("maxEnergy", 2.0);
pset.addParameter<bool>("endcap", false);
pset.addParameter<double>("minTime", -6.);
pset.addParameter<double>("maxTime", 6.);
vpset.emplace_back(pset);
}
{
edm::ParameterSet pset;
pset.addParameter<double>("depth", 1.0);
pset.addParameter<double>("minEnergy", 1.0);
pset.addParameter<double>("maxEnergy", 2.0);
pset.addParameter<bool>("endcap", true);
pset.addParameter<double>("minTime", -20.5);
pset.addParameter<double>("maxTime", 20.5);
vpset.emplace_back(pset);
}
{
edm::ParameterSet pset;
pset.addParameter<double>("depth", 1.0);
pset.addParameter<double>("minEnergy", 2.0);
pset.addParameter<double>("maxEnergy", 5.0);
pset.addParameter<bool>("endcap", false);
pset.addParameter<double>("minTime", -4.);
pset.addParameter<double>("maxTime", 4.);
vpset.emplace_back(pset);
}
{
edm::ParameterSet pset;
pset.addParameter<double>("depth", 1.0);
pset.addParameter<double>("minEnergy", 2.0);
pset.addParameter<double>("maxEnergy", 5.0);
pset.addParameter<bool>("endcap", true);
pset.addParameter<double>("minTime", -12.);
pset.addParameter<double>("maxTime", 12.);
vpset.emplace_back(pset);
}
{
edm::ParameterSet pset;
pset.addParameter<double>("depth", 1.0);
pset.addParameter<double>("minEnergy", 5.0);
pset.addParameter<double>("maxEnergy", 20.0);
pset.addParameter<bool>("endcap", false);
pset.addParameter<double>("minTime", -4.);
pset.addParameter<double>("maxTime", 4.);
vpset.emplace_back(pset);
}
{
edm::ParameterSet pset;
pset.addParameter<double>("depth", 1.0);
pset.addParameter<double>("minEnergy", 5.0);
pset.addParameter<double>("maxEnergy", 20.0);
pset.addParameter<bool>("endcap", true);
pset.addParameter<double>("minTime", -5.);
pset.addParameter<double>("maxTime", 5.);
vpset.emplace_back(pset);
}
{
edm::ParameterSet pset;
pset.addParameter<double>("depth", 1.0);
pset.addParameter<double>("minEnergy", 20.0);
pset.addParameter<double>("maxEnergy", 1e24);
pset.addParameter<bool>("endcap", false);
pset.addParameter<double>("minTime", -4.);
pset.addParameter<double>("maxTime", 4.);
vpset.emplace_back(pset);
}
{
edm::ParameterSet pset;
pset.addParameter<double>("depth", 1.0);
pset.addParameter<double>("minEnergy", 20.0);
pset.addParameter<double>("maxEnergy", 1e24);
pset.addParameter<bool>("endcap", true);
pset.addParameter<double>("minTime", -5.);
pset.addParameter<double>("maxTime", 5.);
vpset.emplace_back(pset);
}
edm::ParameterSetDescription psd;
psd.add<double>("depth", 1.0);
psd.add<double>("minEnergy", 0.0);
psd.add<double>("maxEnergy", 1e24);
psd.add<bool>("endcap", false);
psd.add<double>("minTime", -50.);
psd.add<double>("maxTime", 50.);
desc.addVPSet("cuts", psd, vpset);
}
descriptions.add("particleFlowClusterECALTimeSelected", desc);
}

using namespace std;
using namespace edm;

Expand Down
33 changes: 33 additions & 0 deletions RecoParticleFlow/PFClusterProducer/plugins/PFMultiDepthClusterProducer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class PFMultiDepthClusterProducer : public edm::stream::EDProducer<> {

void beginRun(const edm::Run&, const edm::EventSetup&) override;
void produce(edm::Event&, const edm::EventSetup&) override;
static void fillDescriptions(edm::ConfigurationDescriptions& descriptions);

private:
// inputs
Expand All @@ -40,6 +41,38 @@ class PFMultiDepthClusterProducer : public edm::stream::EDProducer<> {

DEFINE_FWK_MODULE(PFMultiDepthClusterProducer);

void PFMultiDepthClusterProducer::fillDescriptions(edm::ConfigurationDescriptions& descriptions) {
edm::ParameterSetDescription desc;
desc.add<edm::InputTag>("clustersSource", {});
desc.add<edm::ParameterSetDescription>("energyCorrector", {});
{
edm::ParameterSetDescription pset0;
pset0.add<std::string>("algoName", "PFMultiDepthClusterizer");
{
edm::ParameterSetDescription pset1;
pset1.add<std::string>("algoName", "Basic2DGenericPFlowPositionCalc");
{
edm::ParameterSetDescription psd;
psd.add<std::vector<int>>("depths", {});
psd.add<std::string>("detector");
psd.add<std::vector<double>>("logWeightDenominator", {});
pset1.addVPSet("logWeightDenominatorByDetector", psd, {});
}
pset1.add<double>("minAllowedNormalization", 1e-09);
pset1.add<double>("minFractionInCalc", 1e-09);
pset1.add<int>("posCalcNCrystals", -1);
pset0.add<edm::ParameterSetDescription>("allCellsPositionCalc", pset1);
}
pset0.add<double>("minFractionToKeep", 1e-07);
pset0.add<double>("nSigmaEta", 2.0);
pset0.add<double>("nSigmaPhi", 2.0);
desc.add<edm::ParameterSetDescription>("pfClusterBuilder", pset0);
}
desc.add<edm::ParameterSetDescription>("positionReCalc", {});
desc.add<bool>("usePFThresholdsFromDB", false);
descriptions.addWithDefaultLabel(desc);
}

#ifdef PFLOW_DEBUG
#define LOGVERB(x) edm::LogVerbatim(x)
#define LOGWARN(x) edm::LogWarning(x)
Expand Down
9 changes: 5 additions & 4 deletions RecoParticleFlow/PFClusterProducer/plugins/PFRecHitProducer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,10 @@ void PFRecHitProducer::produce(edm::Event& iEvent, const edm::EventSetup& iSetup
}

void PFRecHitProducer::fillDescriptions(edm::ConfigurationDescriptions& descriptions) {
//The following says we do not know what parameters are allowed so do no validation
// Please change this to state exactly what you do use, even if it is no parameters
edm::ParameterSetDescription desc;
desc.setUnknown();
descriptions.addDefault(desc);
edm::ParameterSetDescription psd;
psd.setUnknown();
Copy link
Contributor

Choose a reason for hiding this comment

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

Using setUknown really defeats the purpose of the fillDescriptions.

Copy link
Contributor Author

@stahlleiton stahlleiton Jun 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The issue I had in this particular class was that there were so many variations of this parameter set (in different python configuration files scattered across CMSSW), that failed to find a super set that cover all (so end up declaring it as unknown). Same with PFClusterProducer. I would have to think if there is a way to avoid having a long list of optional parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The unknown parameter set descriptions are removed in 2da72f0

Copy link
Contributor

@mmusich mmusich Jun 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@stahlleiton just a word of caution about the (many) addOptional added in 2da72f0. Currently the ConfDB parsing skips any cms.optional in the resulting cfi files, so if at some point any of these need to be configured at HLT it won't be possible (unless a default is provided in the C++ source code).

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 made them optional because there were parameter sets with different content used in different parts of CMSSW. So I only used add for those parameters that were common among all parameter sets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the optional condition from all the parameters I added in the fillDescription in c85415b

desc.add<edm::ParameterSetDescription>("navigator", psd);
desc.addVPSet("producers", psd, {});
descriptions.addWithDefaultLabel(desc);
}
Loading