Skip to content

Commit

Permalink
apply more review comments
Browse files Browse the repository at this point in the history
From comments by @guitargeek and @hageboeck on PR #8567 (#8567):

- Simplify copy constructor.
- Inline simple getter/setters.
- Replace fabs with std::abs.
  • Loading branch information
egpbos committed Jul 15, 2021
1 parent 46ff604 commit cfe1cad
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 47 deletions.
15 changes: 7 additions & 8 deletions math/minuit2/inc/Minuit2/NumericalDerivator.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ class NumericalDerivator {
const DerivatorElement &previous);

double GetValue() const { return fVal; }
void SetStepTolerance(double value);
void SetGradTolerance(double value);
void SetNCycles(unsigned int value);
void SetErrorLevel(double value);
inline void SetStepTolerance(double value) { fStepTolerance = value; }
inline void SetGradTolerance(double value) { fGradTolerance = value; }
inline void SetNCycles(unsigned int value) { fNCycles = value; }
inline void SetErrorLevel(double value) { fUp = value; }

double Int2ext(const ROOT::Fit::ParameterSettings &parameter, double val) const;
double Ext2int(const ROOT::Fit::ParameterSettings &parameter, double val) const;
Expand All @@ -74,9 +74,9 @@ class NumericalDerivator {
const std::vector<ROOT::Fit::ParameterSettings> &parameters,
std::vector<DerivatorElement> &gradient);

bool AlwaysExactlyMimicMinuit2() const;
void SetAlwaysExactlyMimicMinuit2(bool flag);
inline bool AlwaysExactlyMimicMinuit2() const { return fAlwaysExactlyMimicMinuit2; }
inline void SetAlwaysExactlyMimicMinuit2(bool flag) { fAlwaysExactlyMimicMinuit2 = flag; }

private:
double fStepTolerance = 0.5;
double fGradTolerance = 0.1;
Expand All @@ -100,7 +100,6 @@ class NumericalDerivator {
unsigned int fNCycles = 2;
bool fAlwaysExactlyMimicMinuit2;


};

std::ostream &operator<<(std::ostream &out, const DerivatorElement &value);
Expand Down
42 changes: 3 additions & 39 deletions math/minuit2/src/NumericalDerivator.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -49,34 +49,8 @@ NumericalDerivator::NumericalDerivator(double step_tolerance, double grad_tolera
{
}

// deep copy constructor
NumericalDerivator::NumericalDerivator(const NumericalDerivator &other)
: fStepTolerance(other.fStepTolerance), fGradTolerance(other.fGradTolerance), fNCycles(other.fNCycles),
fUp(other.fUp), fVal(other.fVal), fVx(other.fVx), fVxExternal(other.fVxExternal), fDfmin(other.fDfmin),
fVrysml(other.fVrysml), fPrecision(other.fPrecision), fAlwaysExactlyMimicMinuit2(other.fAlwaysExactlyMimicMinuit2),
fVxFValCache(other.fVxFValCache)
{
}

void NumericalDerivator::SetStepTolerance(double value)
{
fStepTolerance = value;
}

void NumericalDerivator::SetGradTolerance(double value)
{
fGradTolerance = value;
}

void NumericalDerivator::SetNCycles(unsigned int value)
{
fNCycles = value;
}
NumericalDerivator::NumericalDerivator(const NumericalDerivator &other) = default;

void NumericalDerivator::SetErrorLevel(double value)
{
fUp = value;
}

/// This function sets internal state based on input parameters. This state
/// setup is used in the actual (partial) derivative calculations.
Expand Down Expand Up @@ -292,9 +266,9 @@ void NumericalDerivator::SetInitialGradient(const ROOT::Math::IBaseFunctionMulti

var2 = Ext2int(*parameter, sav2);
double vmin = var2 - var;
double gsmin = 8. * eps2 * (fabs(var) + eps2);
double gsmin = 8. * eps2 * (std::abs(var) + eps2);
// protect against very small step sizes which can cause dirin to zero and then nan values in grd
double dirin = std::max(0.5 * (fabs(vplu) + fabs(vmin)), gsmin);
double dirin = std::max(0.5 * (std::abs(vplu) + std::abs(vmin)), gsmin);
double g2 = 2.0 * fUp / (dirin * dirin);
double gstep = std::max(gsmin, 0.1 * dirin);
double grd = g2 * dirin;
Expand All @@ -309,16 +283,6 @@ void NumericalDerivator::SetInitialGradient(const ROOT::Math::IBaseFunctionMulti
}
}

bool NumericalDerivator::AlwaysExactlyMimicMinuit2() const
{
return fAlwaysExactlyMimicMinuit2;
};

void NumericalDerivator::SetAlwaysExactlyMimicMinuit2(bool flag)
{
fAlwaysExactlyMimicMinuit2 = flag;
}

std::ostream &operator<<(std::ostream &out, const DerivatorElement &value)
{
return out << "(derivative: " << value.derivative << ", second_derivative: " << value.second_derivative
Expand Down

0 comments on commit cfe1cad

Please sign in to comment.