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.

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

L1Nano addition #40663

Merged
merged 12 commits into from
Apr 1, 2023
Next Next commit
initial version of L1Nano
  • Loading branch information
Sam-Harper committed Feb 9, 2023
commit 47647baa44fe717fc92e06a1e23416b6be9e2fea
52 changes: 52 additions & 0 deletions PhysicsTools/NanoAOD/interface/SimpleFlatTableProducer.h
Original file line number Diff line number Diff line change
@@ -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"
@@ -349,6 +350,57 @@ 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

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

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");
}
}
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_)) {
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

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

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

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

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:
20 changes: 20 additions & 0 deletions PhysicsTools/NanoAOD/plugins/SimpleFlatTableProducerPlugins.cc
Original file line number Diff line number Diff line change
@@ -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);
@@ -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);
119 changes: 119 additions & 0 deletions PhysicsTools/NanoAOD/python/l1trig_cff.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
import FWCore.ParameterSet.Config as cms
from PhysicsTools.NanoAOD.nano_eras_cff import *
from PhysicsTools.NanoAOD.common_cff import *

l1ObjVars = cms.PSet(
P3Vars,
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")
)


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), # this is the main table for L1 EGs
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

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

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

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

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

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

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

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), # 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=""),
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), # this is the main table for L1 EGs
variables = cms.PSet(PTVars,
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), # this is the main table for L1 EGs
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

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

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)
5 changes: 5 additions & 0 deletions PhysicsTools/NanoAOD/python/nano_cff.py
Original file line number Diff line number Diff line change
@@ -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(
@@ -210,3 +211,7 @@ 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)
return process