-
Notifications
You must be signed in to change notification settings - Fork 1.5k
RooFit::MultiProcess & TestStatistics part 5b: test RooGradMinimizerFcn #8694
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
Changes from all commits
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 | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -215,6 +215,14 @@ namespace ROOT { | |||||||||
| Return the partial derivative with respect to the passed coordinate | ||||||||||
| */ | ||||||||||
| T Derivative(const T *x, unsigned int icoord = 0) const { return DoDerivative(x, icoord); } | ||||||||||
| /// In some cases, the derivative algorithm will use information from the previous step, these can be passed | ||||||||||
| /// in with this overload. The `previous_*` arrays can also be used to return second derivative and step size | ||||||||||
| /// so that these can be passed forward again as well at the call site, if necessary. | ||||||||||
| T Derivative(const T *x, unsigned int icoord, T *previous_grad, T *previous_g2, | ||||||||||
| T *previous_gstep) const | ||||||||||
| { | ||||||||||
| return DoDerivative(x, icoord, previous_grad, previous_g2, previous_gstep); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| Optimized method to evaluate at the same time the function value and derivative at a point x. | ||||||||||
|
|
@@ -232,6 +240,14 @@ namespace ROOT { | |||||||||
| function to evaluate the derivative with respect each coordinate. To be implemented by the derived class | ||||||||||
| */ | ||||||||||
| virtual T DoDerivative(const T *x, unsigned int icoord) const = 0; | ||||||||||
| /// In some cases, the derivative algorithm will use information from the previous step, these can be passed | ||||||||||
| /// in with this overload. The `previous_*` arrays can also be used to return second derivative and step size | ||||||||||
| /// so that these can be passed forward again as well at the call site, if necessary. | ||||||||||
| virtual T DoDerivative(const T *x, unsigned int icoord, T * /*previous_grad*/, T * /*previous_g2*/, | ||||||||||
| T * /*previous_gstep*/) const | ||||||||||
|
Comment on lines
+246
to
+247
Contributor
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.
Suggested change
What about giving these new functions a different name? There is no reason to also name it If a shorter but also descriptive name other than |
||||||||||
| { | ||||||||||
| return DoDerivative(x, icoord); | ||||||||||
| }; | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| //___________________________________________________________________________________ | ||||||||||
|
|
@@ -346,6 +362,16 @@ namespace ROOT { | |||||||||
| grad[icoord] = BaseGrad::Derivative(x, icoord); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /// In some cases, the gradient algorithm will use information from the previous step, these can be passed | ||||||||||
| /// in with this overload. The `previous_*` arrays can also be used to return second derivative and step size | ||||||||||
| /// so that these can be passed forward again as well at the call site, if necessary. | ||||||||||
| virtual void Gradient(const T *x, T *grad, T *previous_grad, T *previous_g2, T *previous_gstep) const | ||||||||||
|
Contributor
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.
Suggested change
See equivalent comment about |
||||||||||
| { | ||||||||||
| unsigned int ndim = NDim(); | ||||||||||
| for (unsigned int icoord = 0; icoord < ndim; ++icoord) | ||||||||||
| grad[icoord] = BaseGrad::Derivative(x, icoord, previous_grad, previous_g2, previous_gstep); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| using BaseFunc::NDim; | ||||||||||
|
|
||||||||||
| /** | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -54,6 +54,18 @@ class FCNGradAdapter : public FCNGradientBase { | |||||||||
| }); | ||||||||||
| return fGrad; | ||||||||||
| } | ||||||||||
| std::vector<double> Gradient(const std::vector<double> &v, double *previous_grad, double *previous_g2, | ||||||||||
| double *previous_gstep) const override | ||||||||||
|
Comment on lines
+57
to
+58
Contributor
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.
Suggested change
|
||||||||||
| { | ||||||||||
| fFunc.Gradient(&v[0], &fGrad[0], previous_grad, previous_g2, previous_gstep); | ||||||||||
|
|
||||||||||
| MnPrint("FCNGradAdapter").Debug([&](std::ostream &os) { | ||||||||||
| os << "gradient in FCNAdapter = {"; | ||||||||||
| for (unsigned int i = 0; i < fGrad.size(); ++i) | ||||||||||
| os << fGrad[i] << (i == fGrad.size() - 1 ? '}' : '\t'); | ||||||||||
| }); | ||||||||||
| return fGrad; | ||||||||||
| } | ||||||||||
| // forward interface | ||||||||||
| // virtual double operator()(int npar, double* params,int iflag = 4) const; | ||||||||||
| bool CheckGradient() const override { return false; } | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -41,6 +41,8 @@ class FCNGradientBase : public FCNBase { | |||||||||||
| virtual ~FCNGradientBase() {} | ||||||||||||
|
|
||||||||||||
| virtual std::vector<double> Gradient(const std::vector<double> &) const = 0; | ||||||||||||
| virtual std::vector<double> Gradient(const std::vector<double> ¶meters, double */*previous_grad*/, double */*previous_g2*/, | ||||||||||||
| double */*previous_gstep*/) const { return Gradient(parameters); }; | ||||||||||||
|
Comment on lines
+44
to
+45
Contributor
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.
Suggested change
Besides the name change, try to not have the character sequence |
||||||||||||
|
|
||||||||||||
| virtual bool CheckGradient() const { return true; } | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
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.