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

[HCAL] Mahi for HBHE local reconstruction #21664

Merged
merged 30 commits into from Dec 21, 2017
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
9958112
initial commit for mahi
jaylawhorn Dec 7, 2017
abe1ca8
update to use effective pedestals
jaylawhorn Dec 8, 2017
160b1e9
update HLT customiser with PR#
jaylawhorn Dec 8, 2017
5041bd9
turn mahi on by default
jaylawhorn Dec 8, 2017
b61f6ba
add all mahi parameters to hlt customization
jaylawhorn Dec 8, 2017
c273552
add dynamic pedestals
jaylawhorn Dec 11, 2017
b06b7b5
Add timing information to MAHI
jaylawhorn Dec 13, 2017
a149252
fix naming conventions
jaylawhorn Dec 13, 2017
0022d97
fix naming conventions, remove unneeded variables and comments, chang…
jaylawhorn Dec 13, 2017
538915c
add mutable struct for memory allocation
jaylawhorn Dec 13, 2017
9b72eed
make MahiFit object const
jaylawhorn Dec 13, 2017
f65bb94
change initialization scheme
jaylawhorn Dec 14, 2017
9c6cf0d
clean up
jaylawhorn Dec 14, 2017
617e6f0
add new parameter at hlt
jaylawhorn Dec 14, 2017
4a8c929
fix crash when using 8 TS
jaylawhorn Dec 15, 2017
9574222
do mahi customization for all trigger menus (temporary)
jaylawhorn Dec 15, 2017
314d16f
fix off-by-one error in 2018 scenario (& any with 8TS)
jaylawhorn Dec 18, 2017
2738ad9
naming consistency
jaylawhorn Dec 19, 2017
d5f6a61
clean up pedestal labelling, 8 vs 10 TS switch, naming conventions, b…
jaylawhorn Dec 19, 2017
a9bdfa9
add consistency check, remove unneeded headers
jaylawhorn Dec 19, 2017
c188729
add check that total sum of charge is positive
jaylawhorn Dec 19, 2017
6e1ef7d
turn off method 2/3 at HLT, make the pedestal BX assignment constant
jaylawhorn Dec 19, 2017
f440c0d
make return values match those of Method 2
jaylawhorn Dec 19, 2017
860ffb0
remove commented out line
jaylawhorn Dec 19, 2017
2472ed0
add for comments from david
jaylawhorn Dec 19, 2017
dd984bd
fix filling of chi2
jaylawhorn Dec 20, 2017
69cb8d5
fix index problem, give default values for energy/time/chi2
jaylawhorn Dec 20, 2017
a94b00a
revert default behavior at hlt
jaylawhorn Dec 20, 2017
0827068
turn on method 3 offline
jaylawhorn Dec 20, 2017
b4ded0f
additional customisation for Mahi HLT relval testing
jaylawhorn Dec 20, 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
21 changes: 20 additions & 1 deletion HLTrigger/Configuration/python/customizeHLTforCMSSW.py
Expand Up @@ -17,6 +17,23 @@
# pset.minGoodStripCharge = cms.PSet(refToPSet_ = cms.string('HLTSiStripClusterChargeCutNone'))
# return process

# Add mahi to HCAL local reconstruction
def customiseFor21664(process):
from RecoLocalCalo.HcalRecProducers.HBHEPhase1Reconstructor_cfi import hbheprereco
for producer in producers_by_type(process, "HBHEPhase1Reconstructor"):
producer.algorithm.useM2 = hbheprereco.algorithm.useM2
producer.algorithm.useM3 = hbheprereco.algorithm.useM3
producer.algorithm.useMahi = hbheprereco.algorithm.useMahi
producer.algorithm.dynamicPed = hbheprereco.algorithm.dynamicPed
producer.algorithm.ts4Thresh = hbheprereco.algorithm.ts4Thresh
producer.algorithm.chiSqSwitch = hbheprereco.algorithm.chiSqSwitch
producer.algorithm.activeBXs = hbheprereco.algorithm.activeBXs
producer.algorithm.nMaxItersMin = hbheprereco.algorithm.nMaxItersMin
producer.algorithm.nMaxItersNNLS = hbheprereco.algorithm.nMaxItersNNLS
producer.algorithm.deltaChiSqThresh = hbheprereco.algorithm.deltaChiSqThresh
producer.algorithm.nnlsThresh = hbheprereco.algorithm.nnlsThresh
return process

# Add new parameters to RecoTrackRefSelector
def customiseFor19029(process):
for producer in producers_by_type(process, "RecoTrackRefSelector"):
Expand Down Expand Up @@ -72,6 +89,8 @@ def customizeHLTforCMSSW(process, menuType="GRun"):
process = customiseFor20269(process)
process = customiseFor19989(process)
process = customiseFor20429(process)
process = customiseFor21437(process)
process = customiseFor21437(process)

process = customiseFor21664(process)

return process
4 changes: 4 additions & 0 deletions RecoLocalCalo/HcalRecAlgos/BuildFile.xml
@@ -1,4 +1,6 @@
<use name="boost"/>
<use name="eigen"/>
<use name="clhep"/>
<use name="DataFormats/HcalDigi"/>
<use name="DataFormats/HcalRecHit"/>
<use name="CalibFormats/HcalObjects"/>
Expand All @@ -9,6 +11,8 @@
<use name="FWCore/Framework"/>
<use name="FWCore/PluginManager"/>
<use name="FWCore/ParameterSet"/>
<use name="CondFormats/DataRecord"/>
<use name="vdt_headers"/>
<use name="rootminuit2"/>
<export>
<lib name="1"/>
Expand Down
28 changes: 28 additions & 0 deletions RecoLocalCalo/HcalRecAlgos/interface/EigenMatrixTypes.h
@@ -0,0 +1,28 @@
#ifndef RecoLocalCalo_HcalRecAlgos_EigenMatrixTypes_h
#define RecoLocalCalo_HcalRecAlgos_EigenMatrixTypes_h

#include <Eigen/Dense>
Copy link
Contributor

Choose a reason for hiding this comment

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

a header guard is missing


constexpr int MaxSVSize=10;
constexpr int MaxFSVSize=19;
constexpr int MaxPVSize=10;

Copy link
Contributor

Choose a reason for hiding this comment

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

these are too simple to be in the global space.
Please put in a namespace.
it may make sense to put everything in this file in an e.g. hcal or mahi namespace

Copy link
Contributor

Choose a reason for hiding this comment

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

on a second thought, this namespace fold requirement is not essential in this context.

Copy link
Contributor

Choose a reason for hiding this comment

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

for example, at least one other definition of BXVector exists in CMSSW (for example) - a namespace (or equivalent) is needed for all of this if it needs to be in the interface directory

typedef Eigen::Matrix<double,Eigen::Dynamic,1,0,MaxSVSize,1> SampleVector;
typedef Eigen::Matrix<double,Eigen::Dynamic,1,0,MaxPVSize,1> PulseVector;
typedef Eigen::Matrix<int,Eigen::Dynamic,1,0,MaxPVSize,1> BXVector;

typedef Eigen::Matrix<double,MaxFSVSize,1> FullSampleVector;
typedef Eigen::Matrix<double,MaxFSVSize,MaxFSVSize> FullSampleMatrix;

typedef Eigen::Matrix<double,Eigen::Dynamic,Eigen::Dynamic,0,MaxSVSize,MaxSVSize> SampleMatrix;
typedef Eigen::Matrix<double,Eigen::Dynamic,Eigen::Dynamic,0,MaxPVSize,MaxPVSize> PulseMatrix;
typedef Eigen::Matrix<double,Eigen::Dynamic,Eigen::Dynamic,0,MaxSVSize,MaxPVSize> SamplePulseMatrix;

typedef Eigen::LLT<SampleMatrix> SampleDecompLLT;
typedef Eigen::LLT<PulseMatrix> PulseDecompLLT;
typedef Eigen::LDLT<PulseMatrix> PulseDecompLDLT;

typedef Eigen::Matrix<double,1,1> SingleMatrix;
typedef Eigen::Matrix<double,1,1> SingleVector;

#endif
167 changes: 167 additions & 0 deletions RecoLocalCalo/HcalRecAlgos/interface/MahiFit.h
@@ -0,0 +1,167 @@
#ifndef RecoLocalCalo_HcalRecAlgos_MahiFit_HH
#define RecoLocalCalo_HcalRecAlgos_MahiFit_HH

#include "DataFormats/HcalDetId/interface/HcalSubdetector.h"
#include "DataFormats/HcalDetId/interface/HcalDetId.h"
#include "CalibFormats/HcalObjects/interface/HcalCalibrations.h"
#include "CalibFormats/HcalObjects/interface/HcalCoder.h"
#include "RecoLocalCalo/HcalRecAlgos/interface/EigenMatrixTypes.h"
#include "DataFormats/HcalRecHit/interface/HBHEChannelInfo.h"

#include "CalibCalorimetry/HcalAlgos/interface/HcalPulseShapes.h"
#include "CalibCalorimetry/HcalAlgos/interface/HcalTimeSlew.h"
#include "RecoLocalCalo/HcalRecAlgos/interface/PulseShapeFunctor.h"

#include <Math/Functor.h>

struct MahiNnlsWorkspace {

unsigned int nPulseTot;
unsigned int tsSize;
unsigned int tsOffset;
unsigned int fullTSOffset;
int bxOffset;
double dt;

//holds active bunch crossings
BXVector bxs;

//holds data samples
SampleVector amplitudes;

//holds inverse covariance matrix
SampleMatrix invCovMat;

//holds diagonal noise terms
SampleVector noiseTerms;

//holds flat pedestal uncertainty
SampleMatrix pedConstraint;

//holds full covariance matrix for a pulse shape
//varied in time
std::array<FullSampleMatrix, MaxPVSize> pulseCovArray;

//holds full pulse shape template
std::array<FullSampleVector, MaxPVSize> pulseShapeArray;

//holds full pulse shape derivatives
std::array<FullSampleVector, MaxPVSize> pulseDerivArray;

//holders for calculating pulse shape & covariance matrices
std::array<double, MaxSVSize> pulseN;
std::array<double, MaxSVSize> pulseM;
std::array<double, MaxSVSize> pulseP;

//holds matrix of pulse shape templates for each BX
SamplePulseMatrix pulseMat;

//holds matrix of pulse shape derivatives for each BX
SamplePulseMatrix pulseDerivMat;

//holds residual vector
PulseVector residuals;

//for FNNLS algorithm
unsigned int nP;
PulseVector ampVec;

PulseVector errVec;
PulseVector ampvecpermtest;

SamplePulseMatrix invcovp;
PulseMatrix aTaMat; // A-transpose A (matrix)
PulseVector aTbVec; // A-transpose b (vector)
PulseVector updateWork; // w (vector)

SampleDecompLLT covDecomp;
SampleMatrix covDecompLinv;
PulseMatrix topleft_work;
PulseDecompLDLT pulseDecomp;

};
Copy link
Contributor

Choose a reason for hiding this comment

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

to follow up on #21664 (comment)

Perhaps an "almost const" solution would be to have data members refactored, e.g. make a state struct (even on heap) with a clear reset call at the end/entry of phase1Apply.

I do not see a reset method in the current implementation.

All other methods called by phase1Apply will be const and operate on this state struct.

This implied that the struct is passed by argument

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, it's kind of covered then, I should have read the implementation in .cc


class MahiFit
{
public:
MahiFit();
~MahiFit() { };

void setParameters(bool iDynamicPed, double iTS4Thresh, double chiSqSwitch,
bool iApplyTimeSlew, HcalTimeSlew::BiasSetting slewFlavor,
double iMeanTime, double iTimeSigmaHPD, double iTimeSigmaSiPM,
const std::vector <int> &iActiveBXs, int iNMaxItersMin, int iNMaxItersNNLS,
double iDeltaChiSqThresh, double iNnlsThresh);

void phase1Apply(const HBHEChannelInfo& channelData,
float& reconstructedEnergy,
float& reconstructedTime,
bool& useTriple,
float& chi2) const;

void doFit(std::array<float,3> &correctedOutput, int nbx) const;

void setPulseShapeTemplate (const HcalPulseShapes::Shape& ps);
void resetPulseShapeTemplate(const HcalPulseShapes::Shape& ps);

typedef BXVector::Index Index;
const HcalPulseShapes::Shape* currentPulseShape_=nullptr;

private:

double minimize() const;
void onePulseMinimize() const;
void updateCov() const;
void updatePulseShape(double itQ, FullSampleVector &pulseShape,
FullSampleVector &pulseDeriv,
FullSampleMatrix &pulseCov) const;
double calculateArrivalTime() const;
double calculateChiSq() const;
void nnls() const;
void resetWorkspace() const;

void nnlsUnconstrainParameter(Index idxp) const;
void nnlsConstrainParameter(Index minratioidx) const;

void solveSubmatrix(PulseMatrix& mat, PulseVector& invec, PulseVector& outvec, unsigned nP) const;

double getSiPMDarkCurrent(double darkCurrent, double fcByPE, double lambda) const;

mutable MahiNnlsWorkspace nnlsWork_;

//hard coded in initializer
const unsigned int fullTSSize_;
const unsigned int fullTSofInterest_;

static constexpr int pedestalBX_ = 100;

// Python-configurables
bool dynamicPed_;
float ts4Thresh_;
float chiSqSwitch_;

bool applyTimeSlew_;
HcalTimeSlew::BiasSetting slewFlavor_;

float meanTime_;
float timeSigmaHPD_;
float timeSigmaSiPM_;

std::vector <int> activeBXs_;

int nMaxItersMin_;
int nMaxItersNNLS_;

float deltaChiSqThresh_;
float nnlsThresh_;

unsigned int bxSizeConf_;
int bxOffsetConf_;

//for pulse shapes
int cntsetPulseShape_;
std::unique_ptr<FitterFuncs::PulseShapeFunctor> psfPtr_;
std::unique_ptr<ROOT::Math::Functor> pfunctor_;

};
#endif
4 changes: 4 additions & 0 deletions RecoLocalCalo/HcalRecAlgos/interface/PulseShapeFunctor.h
Expand Up @@ -39,6 +39,10 @@ namespace FitterFuncs{
double singlePulseShapeFunc( const double *x );
double doublePulseShapeFunc( const double *x );
double triplePulseShapeFunc( const double *x );

void getPulseShape(std::array<double,HcalConst::maxSamples>& fillPulseShape) {
fillPulseShape = pulse_shape_;
}

private:
std::array<float,HcalConst::maxPSshapeBin> pulse_hist;
Expand Down
8 changes: 6 additions & 2 deletions RecoLocalCalo/HcalRecAlgos/interface/SimpleHBHEPhase1Algo.h
Expand Up @@ -12,7 +12,7 @@

#include "RecoLocalCalo/HcalRecAlgos/interface/PulseShapeFitOOTPileupCorrection.h"
#include "RecoLocalCalo/HcalRecAlgos/interface/HcalDeterministicFit.h"

#include "RecoLocalCalo/HcalRecAlgos/interface/MahiFit.h"

class SimpleHBHEPhase1Algo : public AbsHBHEPhase1Algo
{
Expand Down Expand Up @@ -45,7 +45,8 @@ class SimpleHBHEPhase1Algo : public AbsHBHEPhase1Algo
float timeShift,
bool correctForPhaseContainment,
std::unique_ptr<PulseShapeFitOOTPileupCorrection> m2,
std::unique_ptr<HcalDeterministicFit> detFit);
std::unique_ptr<HcalDeterministicFit> detFit,
std::unique_ptr<MahiFit> mahi);

inline ~SimpleHBHEPhase1Algo() override {}

Expand Down Expand Up @@ -101,6 +102,9 @@ class SimpleHBHEPhase1Algo : public AbsHBHEPhase1Algo
// "Metod 3" algorithm
std::unique_ptr<HcalDeterministicFit> hltOOTpuCorr_;

// Mahi algorithm
std::unique_ptr<MahiFit> mahiOOTpuCorr_;

HcalPulseShapes theHcalPulseShapes_;
};

Expand Down