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

Small miniAOD improvements #20367

Merged
merged 6 commits into from Sep 8, 2017
Merged

Small miniAOD improvements #20367

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
863334d
PatCandidates Lepton.h : make the miniPFIsolation() const
gpetruc Sep 4, 2017
68f737b
add dZ from PV to PAT Electron & Muon
gpetruc Sep 4, 2017
78b19b4
Add non-templated version of reduceMantissaToNbitsRounding and suppor…
gpetruc Sep 4, 2017
9f31d22
remove code duplication
gpetruc Sep 4, 2017
d5d893e
code-checks
gpetruc Sep 5, 2017
c5e1951
add in quadrature track dzError and PV zError
gpetruc Sep 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion DataFormats/PatCandidates/interface/Electron.h
View file
Open in desktop
Expand Up @@ -203,7 +203,7 @@ namespace pat {
reco::CandidatePtr sourceCandidatePtr( size_type i ) const;

// ---- embed various impact parameters with errors ----
typedef enum IPTYPE { PV2D = 0, PV3D = 1, BS2D = 2, BS3D = 3, IpTypeSize = 4 } IpType;
typedef enum IPTYPE { PV2D = 0, PV3D = 1, BS2D = 2, BS3D = 3, PVDZ = 4, IpTypeSize = 5 } IpType;
/// Impact parameter wrt primary vertex or beamspot
double dB(IPTYPE type) const;
/// Uncertainty on the corresponding impact parameter
Expand Down
2 changes: 1 addition & 1 deletion DataFormats/PatCandidates/interface/Lepton.h
View file
Open in desktop
Expand Up @@ -193,7 +193,7 @@ namespace pat {
void hcalIsoDeposit(const IsoDeposit &dep) { setIsoDeposit(pat::HcalIso, dep); }
void userIsoDeposit(const IsoDeposit &dep, uint8_t index=0) { setIsoDeposit(IsolationKeys(UserBaseIso + index), dep); }

const PFIsolation& miniPFIsolation() { return miniPFIsolation_; }
const PFIsolation& miniPFIsolation() const { return miniPFIsolation_; }
Copy link
Contributor

Choose a reason for hiding this comment

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

please make a backport for this part to 93X and 92X.

void setMiniPFIsolation(PFIsolation const& iso) { miniPFIsolation_ = iso; }

protected:
Expand Down
2 changes: 1 addition & 1 deletion DataFormats/PatCandidates/interface/Muon.h
View file
Open in desktop
Expand Up @@ -229,7 +229,7 @@ namespace pat {
// IpType defines the type of the impact parameter
typedef enum IPTYPE
{
PV2D = 0, PV3D = 1, BS2D = 2, BS3D = 3, IpTypeSize = 4
PV2D = 0, PV3D = 1, BS2D = 2, BS3D = 3, PVDZ = 4, IpTypeSize = 5
} IpType;
void initImpactParameters(void); // init IP defaults in a constructor
double dB(IPTYPE type) const;
Expand Down
40 changes: 40 additions & 0 deletions DataFormats/PatCandidates/interface/libminifloat.h
View file
Open in desktop
Expand Up @@ -2,6 +2,8 @@
#define libminifloat_h
#include "FWCore/Utilities/interface/thread_safety_macros.h"
#include <cstdint>
#include <cassert>
#include <algorithm>

// ftp://ftp.fox-toolkit.org/pub/fasthalffloatconversion.pdf
class MiniFloatConverter {
Expand Down Expand Up @@ -76,6 +78,44 @@ class MiniFloatConverter {
return conv.flt;
}

class ReduceMantissaToNbitsRounding {
public:
ReduceMantissaToNbitsRounding(int bits) :
shift(23-bits), mask((0xFFFFFFFF >> (shift)) << (shift)),
Copy link
Contributor

Choose a reason for hiding this comment

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

it looks like this duplicates the functionality of inline static float reduceMantissaToNbitsRounding(const float &f)
please remove the other implementation or see how the code duplication can be reduced

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is the solution in 9f31d22 ok for you?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, it looks OK.
Thank you.

test(1 << (shift-1)), maxn((1<<bits)-2) {
assert(bits <= 23); // "max mantissa size is 23 bits"
}
float operator()(float f) const {
constexpr uint32_t low23 = (0x007FFFFF); // mask to keep lowest 23 bits = mantissa
constexpr uint32_t hi9 = (0xFF800000); // mask to keep highest 9 bits = the rest
union { float flt; uint32_t i32; } conv;
conv.flt=f;
if (conv.i32 & test) { // need to round
uint32_t mantissa = (conv.i32 & low23) >> shift;
if (mantissa < maxn) mantissa++;
conv.i32 = (conv.i32 & hi9) | (mantissa << shift);
} else {
conv.i32 &= mask;
}
return conv.flt;
}
private:
const int shift;
const uint32_t mask, test, maxn;
};

inline static float reduceMantissaToNbitsRounding(float f, int bits)
{
return ReduceMantissaToNbitsRounding(bits)(f);
}

template<typename InItr, typename OutItr>
static void reduceMantissaToNbitsRounding(int bits, InItr begin, InItr end, OutItr out)
{
std::transform(begin, end, out, ReduceMantissaToNbitsRounding(bits));
}


inline static float max() {
union { float flt; uint32_t i32; } conv;
conv.i32 = 0x477fe000; // = mantissatable[offsettable[0x1e]+0x3ff]+exponenttable[0x1e]
Expand Down
6 changes: 4 additions & 2 deletions DataFormats/PatCandidates/src/classes_def_objects.xml
View file
Open in desktop
Expand Up @@ -16,7 +16,8 @@
<class name="pat::Lepton<reco::BaseTau>" />

<!-- PAT Objects, and embedded data -->
<class name="pat::Electron" ClassVersion="36">
<class name="pat::Electron" ClassVersion="37">
<version ClassVersion="37" checksum="4284869321"/>
<version ClassVersion="36" checksum="199321903"/>
<version ClassVersion="35" checksum="482655666"/>
<version ClassVersion="34" checksum="3720919820"/>
Expand Down Expand Up @@ -59,7 +60,8 @@
</ioread>


<class name="pat::Muon" ClassVersion="21">
<class name="pat::Muon" ClassVersion="22">
<version ClassVersion="22" checksum="1816533594"/>
<version ClassVersion="21" checksum="1539691612"/>
<version ClassVersion="20" checksum="357097717"/>
<version ClassVersion="19" checksum="2754486523"/>
Expand Down
3 changes: 3 additions & 0 deletions PhysicsTools/PatAlgos/plugins/PATElectronProducer.cc
View file
Open in desktop
Expand Up @@ -1174,6 +1174,9 @@ void PATElectronProducer::embedHighLevel( pat::Electron & anElectron,
d0_corr = result.second.value();
d0_err = beamspotIsValid ? result.second.error() : -1.0;
anElectron.setDB( d0_corr, d0_err, pat::Electron::BS3D);

// PVDZ
anElectron.setDB( track->dz(primaryVertex.position()), track->dzError(), pat::Electron::PVDZ );
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't this include the PV z error?
All of the tip [d0] variants above have it.

Copy link
Contributor

Choose a reason for hiding this comment

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

based on the PR description

save dz(PV) into the pat Electron & Muon for convenience of applying it downstream without a PV object

it sounds like the PV error on z should be added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For d0 and ip3d, the b-tag tool does the right error propagation but I don't think it's implemented for dz.
I could do something simple like std::hypot(track->dzError(), primaryVertex.zError()) , even though I don't think it's really the right math.

Copy link
Contributor

Choose a reason for hiding this comment

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

it seems good enough. It's used in a few places (e.g. CommonTools/ParticleFlow/interface/IPCutPFCandidateSelectorDefinition.h, Alignment/OfflineValidation/plugins/PrimaryVertexValidation.cc, DQM/TrackingMonitor/src/TrackAnalyzer.cc, and at least as many more, based on git grep)

perhaps a more complete would be to deconstruct the Track::dz method and propagate all relevant uncertainties, (to be added to TrackBase? or to the IPTools)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pushed the simple hypot version

}

#include "FWCore/Framework/interface/MakerMacros.h"
Expand Down
4 changes: 4 additions & 0 deletions PhysicsTools/PatAlgos/plugins/PATMuonProducer.cc
View file
Open in desktop
Expand Up @@ -701,6 +701,10 @@ void PATMuonProducer::embedHighLevel( pat::Muon & aMuon,
d0_corr = result.second.value();
d0_err = beamspotIsValid ? result.second.error() : -1.0;
aMuon.setDB( d0_corr, d0_err, pat::Muon::BS3D);


// PVDZ
aMuon.setDB( track->dz(primaryVertex.position()), track->dzError(), pat::Muon::PVDZ );
}

#include "FWCore/Framework/interface/MakerMacros.h"
Expand Down