Skip to content

L1Nano addition #40663

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
cmsbuild merged 12 commits into from
Apr 1, 2023
Merged

L1Nano addition #40663

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
47647ba
initial version of L1Nano
Sam-Harper Feb 1, 2023
5a81f6e
adding more customisation options for nano
Sam-Harper Feb 9, 2023
8b71305
adding fill desc and reduced L1Nano option
Sam-Harper Feb 9, 2023
7d30f14
updating format to that suggested by Laurent T
Sam-Harper Feb 14, 2023
a773bdb
adjusting pt/eta/phi precision, adding ET types 2 and 3
Sam-Harper Feb 16, 2023
e003ff2
adding missing variables from reduced as found by Finn
Sam-Harper Mar 2, 2023
915dd4b
code format
Sam-Harper Mar 8, 2023
1b966d0
review comments; bx value and precision
Sam-Harper Mar 23, 2023
14e0837
adjusting to precision=12 and int16
Sam-Harper Mar 23, 2023
450027d
code checks
Sam-Harper Mar 23, 2023
e0a10fc
removing debug statement
Sam-Harper Mar 24, 2023
e1c084e
adding relval test for prompt l1 nano
Sam-Harper Mar 28, 2023
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
67 changes: 67 additions & 0 deletions PhysicsTools/NanoAOD/interface/SimpleFlatTableProducer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "DataFormats/Common/interface/ValueMap.h"
#include "DataFormats/NanoAOD/interface/FlatTable.h"
#include "Utilities/General/interface/ClassName.h"
#include "DataFormats/L1Trigger/interface/BXVector.h"

#include "CommonTools/Utils/interface/StringCutObjectSelector.h"
#include "CommonTools/Utils/interface/StringObjectFunction.h"
Expand Down Expand Up @@ -349,6 +350,72 @@ class SimpleFlatTableProducer : public SimpleFlatTableProducerBase<T, edm::View<
std::vector<std::unique_ptr<ExtVariable<T>>> extvars_;
};

template <typename T>
class BXVectorSimpleFlatTableProducer : public SimpleFlatTableProducerBase<T, BXVector<T>> {
Copy link
Contributor

@vlimant vlimant Feb 3, 2023

Choose a reason for hiding this comment

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

please add a fillDescription to this new module,

Copy link
Contributor Author

@Sam-Harper Sam-Harper Feb 3, 2023

Choose a reason for hiding this comment

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

of course, I didnt do it as it hadnt been done for the others but thats because I was basing of 12_4_X, I see its all done now in the main branch so I can easily make that change.

public:
BXVectorSimpleFlatTableProducer(edm::ParameterSet const &params)
: SimpleFlatTableProducerBase<T, BXVector<T>>(params),
maxLen_(params.existsAs<unsigned int>("maxLen") ? params.getParameter<unsigned int>("maxLen")
: std::numeric_limits<unsigned int>::max()),
cut_(params.getParameter<std::string>("cut"), true),
minBX_(params.getParameter<int>("minBX")),
maxBX_(params.getParameter<int>("maxBX")),
bxVarName_("bx") {
edm::ParameterSet const &varsPSet = params.getParameter<edm::ParameterSet>("variables");
auto varNames = varsPSet.getParameterNamesForType<edm::ParameterSet>();
if (std::find(varNames.begin(), varNames.end(), bxVarName_) != varNames.end()) {
throw cms::Exception("Configuration",
"BXVectorSimpleFlatTableProducer already defines the " + bxVarName_ +
"internally and thus you should not specify it yourself");
}
}

static void fillDescriptions(edm::ConfigurationDescriptions &descriptions) {
edm::ParameterSetDescription desc = SimpleFlatTableProducerBase<T, BXVector<T>>::baseDescriptions();
desc.add<std::string>("cut", "")->setComment(
"selection on the main input collection (but selection can not be bx based)");
desc.addOptional<unsigned int>("maxLen")->setComment(
"define the maximum length of the input collection to put in the branch");
desc.add<int>("minBX", -2)->setComment("min bx (inclusive) to include");
desc.add<int>("maxBX", 2)->setComment("max bx (inclusive) to include");
descriptions.addWithDefaultLabel(desc);
}

std::unique_ptr<nanoaod::FlatTable> fillTable(const edm::Event &iEvent,
const edm::Handle<BXVector<T>> &prod) const override {
std::vector<const T *> selObjs;
std::vector<int> selObjBXs;

if (prod.isValid() || !(this->skipNonExistingSrc_)) {
const int minBX = std::max(minBX_, prod->getFirstBX());
const int maxBX = std::min(maxBX_, prod->getLastBX());
for (int bx = minBX; bx <= maxBX; bx++) {
for (size_t objNr = 0, nrObjs = prod->size(bx); objNr < nrObjs; ++objNr) {
const auto &obj = prod->at(bx, objNr);
if (cut_(obj)) {
selObjs.push_back(&obj);
selObjBXs.push_back(bx);
}
if (selObjs.size() >= maxLen_)
break;
}
}
}
auto out = std::make_unique<nanoaod::FlatTable>(selObjs.size(), this->name_, false, this->extension_);
for (const auto &var : this->vars_)
var->fill(selObjs, *out);
out->template addColumn<int>(bxVarName_, selObjBXs, "BX of the L1 candidate");
Copy link
Contributor

@swertz swertz Mar 10, 2023

Choose a reason for hiding this comment

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

Even if that won't happen in practice, it would be nice if that column was only added if minBX != maxBX

Copy link
Contributor Author

@Sam-Harper Sam-Harper Mar 13, 2023

Choose a reason for hiding this comment

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

so on this, I would like to offer reasoning for always having it in the interests of usability.

For me, it is much easier to know that bx always exist so I can just do bx==0 rather than trying to figure out if the bx variable exists and if so then do bx==0

I think we make things slightly harder for ourselves to have a variable which sometimes exists and sometimes doesnt given the cost of storing said variable is effectively zero as whenever we wouldnt store it would be zero.

However given this is minor, if the above argument doesnt convince you, how about I make it a configurable parameter whether to store the bx when there is only one so we have this option if we dont store all bx in the future. How does that sound?

Copy link
Contributor

@artlbv artlbv Mar 14, 2023

Choose a reason for hiding this comment

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

From experience: it is always better to explicitly require e.g. BX=0 to avoid cases where the file suddenly has more or less BX stored. (it came several times by surprise that the re-emulated objects are only BX0 and thus had seemingly lower occupancy 😅 )
It could be just the smallest possible type, i.e. int8 or so, right?

Copy link
Contributor

@swertz swertz Mar 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

That's all right, I won't insist on it, given that in practice we will store all BXs for central prompt data production. Indeed it can be an int8 (just be aware that int8 is interpreted as a character and sometimes behaves weirdly with ROOT plotting, to avoid that an int16 is also fine).

return out;
}

protected:
const unsigned int maxLen_;
const StringCutObjectSelector<T> cut_;
const int minBX_;
const int maxBX_;
const std::string bxVarName_;
};

template <typename T>
class EventSingletonSimpleFlatTableProducer : public SimpleFlatTableProducerBase<T, T> {
public:
Expand Down
20 changes: 20 additions & 0 deletions PhysicsTools/NanoAOD/plugins/SimpleFlatTableProducerPlugins.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,21 @@ typedef EventSingletonSimpleFlatTableProducer<math::XYZPointF> SimpleXYZPointFla
#include "DataFormats/BeamSpot/interface/BeamSpot.h"
typedef EventSingletonSimpleFlatTableProducer<reco::BeamSpot> SimpleBeamspotFlatTableProducer;

#include "DataFormats/L1Trigger/interface/EGamma.h"
typedef BXVectorSimpleFlatTableProducer<l1t::EGamma> SimpleTriggerL1EGFlatTableProducer;

#include "DataFormats/L1Trigger/interface/Jet.h"
typedef BXVectorSimpleFlatTableProducer<l1t::Jet> SimpleTriggerL1JetFlatTableProducer;

#include "DataFormats/L1Trigger/interface/Tau.h"
typedef BXVectorSimpleFlatTableProducer<l1t::Tau> SimpleTriggerL1TauFlatTableProducer;

#include "DataFormats/L1Trigger/interface/Muon.h"
typedef BXVectorSimpleFlatTableProducer<l1t::Muon> SimpleTriggerL1MuonFlatTableProducer;

#include "DataFormats/L1Trigger/interface/EtSum.h"
typedef BXVectorSimpleFlatTableProducer<l1t::EtSum> SimpleTriggerL1EtSumFlatTableProducer;

#include "FWCore/Framework/interface/MakerMacros.h"
DEFINE_FWK_MODULE(SimpleCandidateFlatTableProducer);
DEFINE_FWK_MODULE(SimpleGenEventFlatTableProducer);
Expand All @@ -33,3 +48,8 @@ DEFINE_FWK_MODULE(SimpleProtonTrackFlatTableProducer);
DEFINE_FWK_MODULE(SimpleLocalTrackFlatTableProducer);
DEFINE_FWK_MODULE(SimpleXYZPointFlatTableProducer);
DEFINE_FWK_MODULE(SimpleBeamspotFlatTableProducer);
DEFINE_FWK_MODULE(SimpleTriggerL1EGFlatTableProducer);
DEFINE_FWK_MODULE(SimpleTriggerL1JetFlatTableProducer);
DEFINE_FWK_MODULE(SimpleTriggerL1MuonFlatTableProducer);
DEFINE_FWK_MODULE(SimpleTriggerL1TauFlatTableProducer);
DEFINE_FWK_MODULE(SimpleTriggerL1EtSumFlatTableProducer);
189 changes: 189 additions & 0 deletions PhysicsTools/NanoAOD/python/l1trig_cff.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
import FWCore.ParameterSet.Config as cms
from PhysicsTools.NanoAOD.nano_eras_cff import *
from PhysicsTools.NanoAOD.common_cff import *

l1PtVars = cms.PSet(
pt = Var("pt", float, precision=10),
phi = Var("phi", float, precision=10),
)
l1P3Vars = cms.PSet(
l1PtVars,
eta = Var("eta", float, precision=10),
)

l1ObjVars = cms.PSet(
l1P3Vars,
hwPt = Var("hwPt()",int,doc="hardware pt"),
hwEta = Var("hwEta()",int,doc="hardware eta"),
hwPhi = Var("hwPhi()",int,doc="hardware phi"),
hwQual = Var("hwQual()",int,doc="hardware qual"),
hwIso = Var("hwIso()",int,doc="hardware iso")
)

l1JetReducedVars = cms.PSet(
l1P3Vars
)

l1EtSumReducedVars = cms.PSet(
l1PtVars,
etSumType = Var("getType()",int,doc="et sum type"),
)
l1EGReducedVars = cms.PSet(
l1P3Vars,
hwIso = Var("hwIso()",int,doc="hardware iso")
)

l1TauReducedVars = cms.PSet(
l1P3Vars,
hwIso = Var("hwIso()",int,doc="hardware iso")
)

l1MuonReducedVars = cms.PSet(
l1P3Vars,
hwQual = Var("hwQual()",int,doc="hardware qual"),
hwCharge = Var("hwCharge()",int,doc=""),
etaAtVtx = Var("etaAtVtx()",float,doc=""),
phiAtVtx = Var("phiAtVtx()",float,doc=""),
ptUnconstrained = Var("ptUnconstrained()",float,doc=""),
hwDXY = Var("hwDXY()",int,doc=""),
)

l1MuTable = cms.EDProducer("SimpleTriggerL1MuonFlatTableProducer",
src = cms.InputTag("gmtStage2Digis","Muon"),
minBX = cms.int32(-2),
maxBX = cms.int32(2),
cut = cms.string(""),
name= cms.string("L1Mu"),
doc = cms.string(""),
extension = cms.bool(False),
variables = cms.PSet(l1ObjVars,
hwCharge = Var("hwCharge()",int,doc=""),
hwChargeValid = Var("hwChargeValid()",int,doc=""),
tfMuonIndex = Var("tfMuonIndex()",int,doc=""),
hwTag = Var("hwTag()",int,doc=""),
hwEtaAtVtx = Var("hwEtaAtVtx()",int,doc=""),
hwPhiAtVtx = Var("hwPhiAtVtx()",int,doc=""),
etaAtVtx = Var("etaAtVtx()",float,doc=""),
Copy link
Contributor

@swertz swertz Mar 10, 2023

Choose a reason for hiding this comment

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

There are still a few floating point variables stored with full precision, can you please have another look at reducing the precision?

Note that also integers can be stored signed or unsigned and with reduced precision (e.g. 8 or 16 bits).

Copy link
Contributor Author

@Sam-Harper Sam-Harper Mar 13, 2023

Choose a reason for hiding this comment

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

thanks, will do.

Copy link
Contributor

@artlbv artlbv Mar 14, 2023

Choose a reason for hiding this comment

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

FYI this was the cff with the datatypes I used for the size studies (I had modified it by e.g. putting more int16 and precision=10, but there are certainly ways to further reduce the precision)
https://gist.github.com/artlbv/d3e71fb9e4c37f5533bbffaecc6dfbcf

Copy link
Contributor

@swertz swertz Mar 15, 2023

Choose a reason for hiding this comment

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

I had a quick look at that but noted that there were still a few floats with full precision; also note that specifying the precision only works for floats, not ints (for integers you can only specify the type [u]int[8|16|].

Copy link
Contributor

@artlbv artlbv Mar 16, 2023

Choose a reason for hiding this comment

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

ok thanks for checking!
Concerning the float precision: what does that actually mean? I found the code where this is defined but I think I still don't get it. Since the L1 float variables are actually all coming originally from HW values they all have a predetermined bit width. E.g. Jet Et is 12 bit (afaik). Does it mean we should set the float corresponding to that to precision=12 ?

adding @eyigitba @lathomas

Copy link
Contributor

@swertz swertz Mar 16, 2023

Choose a reason for hiding this comment

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

All it does is zero out some bits in the mantissa and let the ROOT file compression take care of the zeros.

Since the L1 float variables are actually all coming originally from HW values they all have a predetermined bit width

That's interesting, so in practice they already have limited precision (only a few non-zero bits in the mantissa). In that case setting the precision to any value not smaller than that will have no effect. The only thing to be decided is if you need the full precision coming from the hardware or if a smaller precision can be afforded (though we probably won't gain much by this).

We're planning to support ROOT fixed-range "floating point" storage in Nano, which works differently, but that's still WIP.

Copy link
Contributor

@artlbv artlbv Mar 16, 2023

Choose a reason for hiding this comment

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

let me also add @dinyar just in case I got something wrong :)

phiAtVtx = Var("phiAtVtx()",float,doc=""),
hwIsoSum = Var("hwIsoSum()",int,doc=""),
hwDPhiExtra = Var("hwDPhiExtra()",int,doc=""),
hwDEtaExtra = Var("hwDEtaExtra()",int,doc=""),
hwRank = Var("hwRank()",int,doc=""),
hwPtUnconstrained = Var("hwPtUnconstrained()",int,doc=""),
ptUnconstrained = Var("ptUnconstrained()",float,doc=""),
hwDXY = Var("hwDXY()",int,doc=""),
)
)


l1JetTable = cms.EDProducer("SimpleTriggerL1JetFlatTableProducer",
src = cms.InputTag("caloStage2Digis","Jet"),
minBX = cms.int32(-2),
maxBX = cms.int32(2),
cut = cms.string(""),
name= cms.string("L1Jet"),
doc = cms.string(""),
extension = cms.bool(False),
variables = cms.PSet(l1ObjVars,
towerIEta = Var("towerIEta()",int,doc=""),
towerIPhi = Var("towerIPhi()",int,doc=""),
rawEt = Var("rawEt()",int,doc=""),
seedEt = Var("seedEt()",int,doc=""),
puEt = Var("puEt()",int,doc=""),
puDonutEt0 = Var("puDonutEt(0)",int,doc=""),
puDonutEt1 = Var("puDonutEt(1)",int,doc=""),
puDonutEt2 = Var("puDonutEt(2)",int,doc=""),
puDonutEt3 = Var("puDonutEt(3)",int,doc=""),
)
)

l1TauTable = cms.EDProducer("SimpleTriggerL1TauFlatTableProducer",
src = cms.InputTag("caloStage2Digis","Tau"),
minBX = cms.int32(-2),
maxBX = cms.int32(2),
cut = cms.string(""),
name= cms.string("L1Tau"),
doc = cms.string(""),
extension = cms.bool(False), # this is the main table for L1 EGs
variables = cms.PSet(l1ObjVars,
towerIEta = Var("towerIEta()",int,doc=""),
towerIPhi = Var("towerIPhi()",int,doc=""),
rawEt = Var("rawEt()",int,doc=""),
isoEt = Var("isoEt()",int,doc=""),
nTT = Var("nTT()",int,doc=""),
hasEM = Var("hasEM()",int,doc=""),
isMerged = Var("isMerged()",int,doc=""),

)
)

l1EtSumTable = cms.EDProducer("SimpleTriggerL1EtSumFlatTableProducer",
src = cms.InputTag("caloStage2Digis","EtSum"),
minBX = cms.int32(-2),
maxBX = cms.int32(2),
cut = cms.string(""),
name= cms.string("L1EtSum"),
doc = cms.string(""),
extension = cms.bool(False),
variables = cms.PSet(l1PtVars,
hwPt = Var("hwPt()",int,doc="hardware pt"),
hwPhi = Var("hwPhi()",int,doc="hardware phi"),
etSumType = Var("getType()",int,doc=""),
)
)

l1EGTable = cms.EDProducer("SimpleTriggerL1EGFlatTableProducer",
src = cms.InputTag("caloStage2Digis","EGamma"),
minBX = cms.int32(-2),
maxBX = cms.int32(2),
cut = cms.string(""),
name= cms.string("L1EG"),
doc = cms.string(""),
extension = cms.bool(False),
variables = cms.PSet(l1ObjVars,
towerIEta = Var("towerIEta()",int,doc="tower ieta"),
towerIPhi = Var("towerIPhi()",int,doc="tower iphi"),
rawEt = Var("rawEt()",int,doc="raw et"),
Copy link
Contributor

@aloeliger aloeliger Feb 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

towerIEta, towerIPhi, and rawEt are repeated for l1EG's, l1Tau's and l1Jet's. Could you maybe save a few lines by just making those a PSet too?

Copy link
Contributor Author

@Sam-Harper Sam-Harper Mar 13, 2023

Choose a reason for hiding this comment

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

thanks for the comment, we've now reworked this file a little and will consider if it still makes sense to do this which it probably does as it means we only have to write the doc string oncey

isoEt = Var("isoEt()",int,doc="iso et"),
footprintEt = Var("footprintEt()",int,doc=" footprint et"),
nTT = Var("nTT()",int,doc="nr trig towers"),
shape = Var("shape()",int,doc="shape"),
towerHoE = Var("towerHoE()",int,doc="tower H/E"),
)
)

l1TablesTask = cms.Task(l1EGTable,l1EtSumTable,l1TauTable,l1JetTable,l1MuTable)

def setL1NanoToReduced(process):
"""
sets the L1 objects only have reduced information which is necessary
for central nano
"""
#reduce the variables to the core variables
#note et sum variables are already reduced
process.l1EGTable.variables = cms.PSet(l1EGReducedVars)
process.l1MuTable.variables = cms.PSet(l1MuonReducedVars)
process.l1JetTable.variables = cms.PSet(l1JetReducedVars)
process.l1TauTable.variables = cms.PSet(l1TauReducedVars)
process.l1EtSumTable.variables = cms.PSet(l1EtSumReducedVars)

#restrict bx
Copy link
Contributor

@swertz swertz Mar 10, 2023

Choose a reason for hiding this comment

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

Following the discussion, we have agreed to store all the BX's also in the reduced table for central prompt data Nano's

Copy link
Contributor Author

@Sam-Harper Sam-Harper Mar 13, 2023

Choose a reason for hiding this comment

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

thanks, will change

process.l1EGTable.minBX = 0
process.l1EGTable.maxBX = 0
process.l1MuTable.minBX = 0
process.l1MuTable.maxBX = 0
process.l1JetTable.minBX = 0
process.l1JetTable.maxBX = 0
process.l1TauTable.minBX = 0
process.l1TauTable.maxBX = 0
process.l1EtSumTable.minBX = 0
process.l1EtSumTable.maxBX = 0

#apply cuts
process.l1EGTable.cut="pt>=10"
process.l1TauTable.cut="pt>=24"
process.l1JetTable.cut="pt>=30"
process.l1MuTable.cut="pt>=3 && hwQual>=8"
process.l1EtSumTable.cut="(getType==8 || getType==1 || getType==2 || getType==3)"

return process
10 changes: 10 additions & 0 deletions PhysicsTools/NanoAOD/python/nano_cff.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from PhysicsTools.NanoAOD.NanoAODEDMEventContent_cff import *
from PhysicsTools.NanoAOD.fsrPhotons_cff import *
from PhysicsTools.NanoAOD.softActivity_cff import *
from PhysicsTools.NanoAOD.l1trig_cff import *

nanoMetadata = cms.EDProducer("UniqueStringProducer",
strings = cms.PSet(
Expand Down Expand Up @@ -210,3 +211,12 @@ def nanoWmassGenCustomize(process):
etaPrecision="{} ? {} : {}".format(pdgSelection, CandVars.eta.precision.value(), genParticleTable.variables.eta.precision.value())
process.genParticleTable.variables.eta.precision=cms.string(etaPrecision)
return process

def nanoL1TrigObjCustomize(process):
process.nanoTableTaskCommon.add(process.l1TablesTask)
process = setL1NanoToReduced(process)
return process

def nanoL1TrigObjCustomizeFull(process):
process.nanoTableTaskCommon.add(process.l1TablesTask)
return process