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
HBHE: M2 small fixed #21821
HBHE: M2 small fixed #21821
Changes from all commits
baa0e56
3ebc4d2
1180423
b485a2b
8e71602
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,8 +9,8 @@ PulseShapeFitOOTPileupCorrection::PulseShapeFitOOTPileupCorrection() : cntsetPul | |
psfPtr_(nullptr), spfunctor_(nullptr), dpfunctor_(nullptr), tpfunctor_(nullptr), | ||
TSMin_(0), TSMax_(0), vts4Chi2_(0), pedestalConstraint_(false), | ||
timeConstraint_(false), addPulseJitter_(false), applyTimeSlew_(false), | ||
ts4Min_(0), vts4Max_(0), pulseJitter_(0), timeMean_(0), timeSig_(0), pedMean_(0), pedSig_(0), | ||
noise_(0) { | ||
ts4Min_(0), vts4Max_(0), pulseJitter_(0), timeMean_(0), timeSig_(0), pedMean_(0) | ||
{ | ||
hybridfitter = new PSFitter::HybridMinimizer(PSFitter::HybridMinimizer::kMigrad); | ||
iniTimesArr = { {-100,-75,-50,-25,0,25,50,75,100,125} }; | ||
} | ||
|
@@ -25,13 +25,6 @@ double PulseShapeFitOOTPileupCorrection::getSiPMDarkCurrent(double darkCurrent, | |
return sqrt(mu/pow(1-lambda,3)) * fcByPE; | ||
} | ||
|
||
void PulseShapeFitOOTPileupCorrection::setChi2Term( bool isHPD ) { | ||
|
||
if(isHPD) timeSig_ = timeSigHPD_; | ||
else timeSig_ = timeSigSiPM_; | ||
|
||
} | ||
|
||
|
||
void PulseShapeFitOOTPileupCorrection::setPUParams(bool iPedestalConstraint, bool iTimeConstraint,bool iAddPulseJitter, | ||
bool iApplyTimeSlew,double iTS4Min, const std::vector<double> & iTS4Max, | ||
|
@@ -68,17 +61,21 @@ void PulseShapeFitOOTPileupCorrection::setPulseShapeTemplate(const HcalPulseShap | |
|
||
if (!(&ps == currentPulseShape_ && isHPD == isCurrentChannelHPD_)) | ||
{ | ||
setChi2Term(isHPD); | ||
resetPulseShapeTemplate(ps,nSamples); | ||
|
||
// redefine the inverttimeSig2 | ||
if(!isHPD) psfPtr_->setinverttimeSig2(1./(timeSigSiPM_*timeSigSiPM_)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Compute Then, timeSigSiPM_ and timeSigHPD_ do not need to be data member: you could use (e.g.) invertTimeSig2SiPM_ and invertimeSig2HPD_ instead There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @perrotta |
||
else psfPtr_->setinverttimeSig2(1./(timeSigHPD_*timeSigHPD_)); | ||
|
||
currentPulseShape_ = &ps; | ||
isCurrentChannelHPD_ = isHPD; | ||
} | ||
} | ||
|
||
void PulseShapeFitOOTPileupCorrection::resetPulseShapeTemplate(const HcalPulseShapes::Shape& ps, unsigned nSamples) { | ||
++ cntsetPulseShape; | ||
psfPtr_.reset(new FitterFuncs::PulseShapeFunctor(ps,pedestalConstraint_,timeConstraint_,addPulseJitter_,applyTimeSlew_, | ||
pulseJitter_,timeMean_,timeSig_,pedMean_,pedSig_,noise_,nSamples)); | ||
psfPtr_.reset(new FitterFuncs::PulseShapeFunctor(ps,pedestalConstraint_,timeConstraint_,addPulseJitter_, | ||
pulseJitter_,timeMean_,pedMean_,nSamples)); | ||
spfunctor_ = std::unique_ptr<ROOT::Math::Functor>( new ROOT::Math::Functor(psfPtr_.get(),&FitterFuncs::PulseShapeFunctor::singlePulseShapeFunc, 3) ); | ||
dpfunctor_ = std::unique_ptr<ROOT::Math::Functor>( new ROOT::Math::Functor(psfPtr_.get(),&FitterFuncs::PulseShapeFunctor::doublePulseShapeFunc, 5) ); | ||
tpfunctor_ = std::unique_ptr<ROOT::Math::Functor>( new ROOT::Math::Functor(psfPtr_.get(),&FitterFuncs::PulseShapeFunctor::triplePulseShapeFunc, 7) ); | ||
|
@@ -306,7 +303,11 @@ void PulseShapeFitOOTPileupCorrection::phase1Apply(const HBHEChannelInfo& channe | |
psfPtr_->setinvertpedSig2(1./(averagePedSig2GeV)); | ||
|
||
if(channelData.hasTimeInfo()) { | ||
ts4Max_=vts4Max_[1]; ts4Chi2_=vts4Chi2_[1]; | ||
|
||
ts4Chi2_=vts4Chi2_[1]; | ||
if(channelData.id().depth()==1) ts4Max_=vts4Max_[1]; | ||
else ts4Max_=vts4Max_[2]; | ||
|
||
} else { | ||
ts4Max_=vts4Max_[0]; ts4Chi2_=vts4Chi2_[0]; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the commented out parts of code in this HybridMinimizer.cc should be better removed, or alternatively some additional line of comment should be added to explain why they must remain there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this file there are large chunks of code commented out, as they are apparently not needed: they should be better removed. Nonetheless, I will give my "+1" to this PR just in case it can still be allowed (as "bug fix") in 10_0_X: in that case
HybridMinimizer.cc
will be adjusted in a forthcoming pull request. If this PR does not enter 10_0_X, or (even better) if it can be adjusted quickly now, I will remove the "+1" waiting for it