Fixes regression and corresponding tests (#758)

* Reintroduce free function constructor

Should fix BinnedFitExecPolicy test errors

* Actually fail if the test result is wrong

* Remove dumb ifdef guard

hist/hist/inc/TF1.h

@@ -303,6 +303,7 @@ public:
    TF1(const char *name, const char *formula, Double_t xmin = 0, Double_t xmax = 1, EAddToList addToGlobList = EAddToList::kDefault);
    TF1(const char *name, Double_t xmin, Double_t xmax, Int_t npar, Int_t ndim = 1, EAddToList addToGlobList = EAddToList::kDefault);
    TF1(const char *name, Double_t (*fcn)(Double_t *, Double_t *), Double_t xmin = 0, Double_t xmax = 1, Int_t npar = 0, Int_t ndim = 1, EAddToList addToGlobList = EAddToList::kDefault);
+   TF1(const char *name, Double_t (*fcn)(const Double_t *, const Double_t *), Double_t xmin = 0, Double_t xmax = 1, Int_t npar = 0, Int_t ndim = 1, EAddToList addToGlobList = EAddToList::kDefault);
 
    template <class T>
    TF1(const char *name, std::function<T(const T *data, const Double_t *param)> &fcn, Double_t xmin = 0, Double_t xmax = 1, Int_t npar = 0, Int_t ndim = 1, EAddToList addToGlobList = EAddToList::kDefault):
@@ -310,18 +311,17 @@ public:
    {}
 
    ////////////////////////////////////////////////////////////////////////////////
-   /// Constructor using a pointer to real function.
+   /// Constructor using a pointer to function.
    ///
    /// \param npar is the number of free parameters used by the function
    ///
    /// This constructor creates a function of type C when invoked
    /// with the normal C++ compiler.
    ///
-   /// see test program test/stress.cxx (function stress1) for an example.
-   /// note the interface with an intermediate pointer.
    ///
    /// WARNING! A function created with this constructor cannot be Cloned
 
+
    template <class T>
    TF1(const char *name, T(*fcn)(const T *, const Double_t *), Double_t xmin = 0, Double_t xmax = 1, Int_t npar = 0, Int_t ndim = 1, EAddToList addToGlobList = EAddToList::kDefault):
       TF1(EFType::kTemplated, name, xmin, xmax, npar, ndim, addToGlobList, new TF1Parameters(npar), new TF1FunctorPointerImpl<T>(fcn))

hist/hist/src/TF1.cxx

@@ -499,6 +499,22 @@ TF1::TF1(const char *name, Double_t (*fcn)(Double_t *, Double_t *), Double_t xmi
    TF1(EFType::kPtrScalarFreeFcn, name, xmin, xmax, npar, ndim, addToGlobList, new TF1Parameters(npar), new TF1FunctorPointerImpl<double>(ROOT::Math::ParamFunctor(fcn)))
 {}
 
+////////////////////////////////////////////////////////////////////////////////
+/// Constructor using a pointer to real function.
+///
+/// \param npar is the number of free parameters used by the function
+///
+/// This constructor creates a function of type C when invoked
+/// with the normal C++ compiler.
+///
+/// see test program test/stress.cxx (function stress1) for an example.
+/// note the interface with an intermediate pointer.
+///
+/// WARNING! A function created with this constructor cannot be Cloned.
+
+TF1::TF1(const char *name, Double_t (*fcn)(const Double_t *, const Double_t *), Double_t xmin, Double_t xmax, Int_t npar, Int_t ndim, EAddToList addToGlobList) :
+   TF1(EFType::kPtrScalarFreeFcn, name, xmin, xmax, npar, ndim, addToGlobList, new TF1Parameters(npar), new TF1FunctorPointerImpl<double>(ROOT::Math::ParamFunctor(fcn)))
+{}
 
 ////////////////////////////////////////////////////////////////////////////////
 /// Constructor using the Functor class.

math/mathcore/test/fit/testBinnedFitExecPolicy.cxx

@@ -7,13 +7,13 @@
 #include "TError.h"
 #include "Math/MinimizerOptions.h"
 
-int compareResult(double v1, double v2, std::string s = "", double tol = 0.01)
+bool compareResult(double v1, double v2, std::string s = "", double tol = 0.01)
 {
    // compare v1 with reference v2
    //  // give 1% tolerance
-   if (std::abs(v1 - v2) < tol * std::abs(v2)) return 0;
+   if (std::abs(v1 - v2) < tol * std::abs(v2)) return true;
    std::cerr << s << " Failed comparison of fit results \t chi2 = " << v1 << "   it should be = " << v2 << std::endl;
-   return -1;
+   return false;
 }
 
 template <class T>
@@ -47,6 +47,7 @@ int main()
       return -1;
    }
 
+   int correctness = 0;
 #ifdef R__USE_IMT
    std::cout << "\n **FIT: Multithreaded Chi2 **\n\n";
    f->SetParameters(1, 1000, 7.5, 1.5);
@@ -55,7 +56,9 @@ int main()
       Error("testBinnedFitExecPolicy", "Multithreaded Chi2 Fit failed!");
       return -1;
    } else {
-      compareResult(r2->MinFcnValue(), r1->MinFcnValue(), "Mutithreaded Chi2 Fit: ");
+     correctness = compareResult(r2->MinFcnValue(), r1->MinFcnValue(), "Mutithreaded Chi2 Fit: ");
+     if(!correctness)
+      return 1;
    }
 
    std::cout << "\n **FIT: Multithreaded Binned Likelihood **\n\n";
@@ -65,7 +68,9 @@ int main()
       Error("testBinnedFitExecPolicy", "Multithreaded Binned Likelihood Fit failed!");
       return -1;
    } else {
-      compareResult(rL2->MinFcnValue(), rL1->MinFcnValue(), "Mutithreaded Binned Likelihood Fit (PoissonLogL): ");
+      correctness = compareResult(rL2->MinFcnValue(), rL1->MinFcnValue(), "Mutithreaded Binned Likelihood Fit (PoissonLogL): ");
+      if(!correctness)
+         return 2;
    }
 #endif
 
@@ -79,7 +84,9 @@ int main()
       Error("testBinnedFitExecPolicy", "Vectorized Chi2 Fit failed!");
       return -1;
    } else {
-      compareResult(r3->MinFcnValue(), r1->MinFcnValue(), "Vectorized Chi2 Fit: ");
+      correctness = compareResult(r3->MinFcnValue(), r1->MinFcnValue(), "Vectorized Chi2 Fit: ");
+      if(!correctness)
+         return 3;
    }
 
    std::cout << "\n **FIT: Vectorized Binned Likelihood **\n\n";
@@ -89,7 +96,9 @@ int main()
       Error("testBinnedFitExecPolicy", "Vectorized Binned Likelihood Fit failed!");
       return -1;
    } else {
-      compareResult(rL3->MinFcnValue(), rL1->MinFcnValue(), "Vectorized Binned Likelihood Fit (PoissonLogL) Fit: ");
+      correctness = compareResult(rL3->MinFcnValue(), rL1->MinFcnValue(), "Vectorized Binned Likelihood Fit (PoissonLogL) Fit: ");
+      if(!correctness)
+         return 4;
    }
 
 #ifdef R__USE_IMT
@@ -99,7 +108,9 @@ int main()
       Error("testBinnedFitExecPolicy", "Mutithreaded vectorized Chi2 Fit failed!");
       return -1;
    } else {
-      compareResult(r4->MinFcnValue(), r1->MinFcnValue(), "Mutithreaded vectorized Chi2 Fit: ");
+      correctness = compareResult(r4->MinFcnValue(), r1->MinFcnValue(), "Mutithreaded vectorized Chi2 Fit: ");
+      if(!correctness)
+         return 5;
    }
 
    std::cout << "\n **FIT: Multithreaded and vectorized Binned Likelihood **\n\n";
@@ -109,10 +120,13 @@ int main()
       Error("testBinnedFitExecPolicy", "Multithreaded Binned Likelihood vectorized Fit failed!");
       return -1;
    } else {
-      compareResult(rL4->MinFcnValue(), rL1->MinFcnValue(),
+      correctness = compareResult(rL4->MinFcnValue(), rL1->MinFcnValue(),
                     "Mutithreaded vectorized Binned Likelihood Fit (PoissonLogL) Fit: ");
+      if(!correctness)
+         return 6;
    }
 
 #endif
 #endif
+   return 0;
 }

math/mathcore/test/fit/testLogLExecPolicy.cxx

@@ -10,13 +10,13 @@
 
 constexpr int paramSize = 6;
 
-int compareResult(double v1, double v2, std::string s = "", double tol = 0.01)
+bool compareResult(double v1, double v2, std::string s = "", double tol = 0.01)
 {
    // compare v1 with reference v2
    //  // give 1% tolerance
-   if (std::abs(v1 - v2) < tol * std::abs(v2)) return 0;
+   if (std::abs(v1 - v2) < tol * std::abs(v2)) return true;
    std::cerr << s << " Failed comparison of fit results \t logl = " << v1 << "   it should be = " << v2 << std::endl;
-   return -1;
+   return false;
 }
 
 //Functor for a Higgs Fit normalized with analytical integral
@@ -220,6 +220,7 @@ private:
 int main()
 {
 
+   bool correctness;
    TestVector test(200000);
 
    //Sequential
@@ -227,19 +228,19 @@ int main()
       Error("testLogLExecPolicy", "Fit failed!");
       return -1;
    }
-#if defined(R__USE_IMT) && defined(R__HAS_VECCORE)
    auto seq = test.GetFitter().Result().MinFcnValue();
-#endif
 
-// #ifdef R__USE_IMT
-//    //Multithreaded
-//    if (!test.testMTFit()) {
-//       Error("testLogLExecPolicy", "Multithreaded Fit failed!");
-//       return -1;
-//    }
-//    auto seqMT = test.GetFitter().Result().MinFcnValue();
-//    compareResult(seqMT, seq, "Mutithreaded LogL Fit: ");
-// #endif
+#ifdef R__USE_IMT
+   //Multithreaded
+   if (!test.testMTFit()) {
+      Error("testLogLExecPolicy", "Multithreaded Fit failed!");
+      return -1;
+   }
+   auto seqMT = test.GetFitter().Result().MinFcnValue();
+   correctness = compareResult(seqMT, seq, "Mutithreaded LogL Fit: ");
+   if(!correctness)
+         return 1;
+#endif
 
 #ifdef R__HAS_VECCORE
    //Vectorized
@@ -248,17 +249,21 @@ int main()
       return -1;
    }
    auto vec = test.GetFitter().Result().MinFcnValue();
-   compareResult(vec, seq, "vectorized LogL Fit: ");
-
-// #ifdef R__USE_IMT
-//    //Multithreaded and vectorized
-//    if (!test.testMTFitVec()) {
-//       Error("testLogLExecPolicy", "Multithreaded + vectorized Fit failed!");
-//       return -1;
-//    }
-//    auto vecMT = test.GetFitter().Result().MinFcnValue();
-//    compareResult(vecMT, seq, "Mutithreaded + vectorized LogL Fit: ");
-// #endif
+   correctness = compareResult(vec, seq, "vectorized LogL Fit: ");
+   if(!correctness)
+         return 2;
+
+#ifdef R__USE_IMT
+   //Multithreaded and vectorized
+   if (!test.testMTFitVec()) {
+      Error("testLogLExecPolicy", "Multithreaded + vectorized Fit failed!");
+      return -1;
+   }
+   auto vecMT = test.GetFitter().Result().MinFcnValue();
+   correctness = compareResult(vecMT, seq, "Mutithreaded + vectorized LogL Fit: ");
+   if(!correctness)
+         return 3;
+#endif
 #endif
 
 //    //Multiprocessed