[Math] Make building old C++ Minuit implementation optional

Minuit 2 has been the default for 2 years, and one day we should
consider not building ROOT with the old C++ TMinuit implementation
anymore.

To make it possible to try this out, introduce a new `minuit1` flag that
is on by default.

Building with `minuit1=OFF` will also help to ensure that we are
actually using Minuit 2 in all out tests. With this, it was uncovered
that the `stressRooFit` actually didn't use Minuit 2, even thought it's
the default minimizer in RooFit as well. Therefore, the default
minimizer setting in `stressRooFit`/`stressRooStats` are changed to
`Minuit2`, reference files are updated, and new tests with the old
Minuit 1 are added to keep previous test coverage.

Author: guitargeek

cmake/modules/RootBuildOptions.cmake

@@ -146,6 +146,7 @@ ROOT_BUILD_OPTION(llvm13_broken_tests OFF "Enable broken tests with LLVM 13 on W
 ROOT_BUILD_OPTION(macos_native OFF "Disable looking for libraries, includes and binaries in locations other than a native installation (MacOS only)")
 ROOT_BUILD_OPTION(mathmore OFF "Build libMathMore extended math library (requires GSL) [GPL]")
 ROOT_BUILD_OPTION(memory_termination OFF "Free internal ROOT memory before process termination (experimental, used for leak checking)")
+ROOT_BUILD_OPTION(minuit1 ON "Build also the original C++ implementation of Minuit, alongside Minuit 2. This flag has no effect on Minuit 2, which is always built.")
 ROOT_BUILD_OPTION(minuit2_mpi OFF "Enable support for MPI in Minuit2")
 ROOT_BUILD_OPTION(minuit2_omp OFF "Enable support for OpenMP in Minuit2")
 ROOT_BUILD_OPTION(mpi OFF "Enable support for Message Passing Interface (MPI)")

math/CMakeLists.txt

@@ -9,7 +9,9 @@ if(mathmore)
   add_subdirectory(mathmore)
 endif()
 add_subdirectory(matrix)
-add_subdirectory(minuit)
+if(minuit1)
+  add_subdirectory(minuit)
+endif()
 add_subdirectory(minuit2)
 add_subdirectory(fumili)
 add_subdirectory(physics)

roofit/roofitcore/CMakeLists.txt

@@ -492,6 +492,10 @@ if(fftw3)
   target_compile_definitions(RooFitCore PUBLIC ROOFIT_MATH_FFTW3)
 endif()
 
+if(minuit1)
+  target_compile_definitions(RooFitCore PUBLIC ROOFIT_MINUIT_1)
+endif()
+
 target_include_directories(RooFitCore PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/res>)
 
 # The SMatrix package is header-only, but it's only exposed via the Smatrix

roofit/roofitcore/test/CMakeLists.txt

@@ -90,6 +90,10 @@ if(mathmore)
 endif()
 
 configure_file(stressRooFit_ref.root stressRooFit_ref.root COPYONLY)
+configure_file(stressRooFit_ref_minuit1.root stressRooFit_ref_minuit1.root COPYONLY)
+if(minuit1)
+  ROOT_ADD_TEST(test-stressroofit-cpu-minuit1 COMMAND stressRooFit -minim Minuit -f stressRooFit_ref_minuit1.root -b cpu FAILREGEX "FAILED|Error in")
+endif()
 if(roofit_legacy_eval_backend)
   ROOT_ADD_TEST(test-stressroofit-legacy COMMAND stressRooFit -b legacy FAILREGEX "FAILED|Error in")
 endif()

roofit/roofitcore/test/stressRooFit.cxx

@@ -255,7 +255,7 @@ int main(int argc, const char *argv[])
 
    // string refFileName = "http://root.cern.ch/files/stressRooFit_v534_ref.root" ;
    string refFileName = "stressRooFit_ref.root";
-   string minimizerName = "Minuit";
+   string minimizerName = "Minuit2";
 
    auto verbosityOptionErrorMsg = "Multiple verbosity-related options have been passed to stressRooFit! The options "
                                   "-v, -vv, and -q are mutually exclusive.";

roofit/roofitcore/test/stressRooFit_ref.root

@@ -49,7 +49,6 @@ ROOT_STANDARD_LIBRARY_PACKAGE(RooFitMore
     Hist
     Matrix
     Tree
-    Minuit
     RIO
     MathCore
     Foam

roofit/roofitcore/test/stressRooFit_ref_minuit1.root

@@ -9,6 +9,10 @@
 # @author Pere Mato, CERN
 ############################################################################
 
+if(minuit1)
+  list(APPEND EXTRA_LIBRARIES Minuit)
+endif()
+
 set (EXTRA_DICT_OPTS)
 if (runtime_cxxmodules AND WIN32)
   set (EXTRA_DICT_OPTS NO_CXXMODULE)
@@ -121,6 +125,8 @@ ROOT_STANDARD_LIBRARY_PACKAGE(RooStats
     src/UpperLimitMCSModule.cxx
   DICTIONARY_OPTIONS
     "-writeEmptyRootPCM"
+  LIBRARIES
+    ${EXTRA_LIBRARIES}
   DEPENDENCIES
     Core
     RooFit
@@ -130,7 +136,6 @@ ROOT_STANDARD_LIBRARY_PACKAGE(RooStats
     Hist
     Matrix
     MathCore
-    Minuit
     Foam
     Graf
     Gpad

roofit/roofitmore/CMakeLists.txt

@@ -58,7 +58,9 @@
 #include "RooFunctor.h"
 #include "RooProfileLL.h"
 
+#ifdef ROOFIT_MINUIT_1
 #include "TMinuitMinimizer.h"
+#endif
 
 #include <string>
 #include <algorithm>
@@ -260,8 +262,10 @@ bool LikelihoodInterval::CreateMinimizer() {
       ccoutE(InputArguments) << minimType << " is wrong type of minimizer for getting interval limits or contours - must use Minuit or Minuit2" << std::endl;
       return false;
    }
+#ifdef ROOFIT_MINUIT_1
    // do not use static instance of TMInuit which could interfere with RooFit
    if (minimType == "Minuit")  TMinuitMinimizer::UseStaticMinuit(false);
+#endif
    // create minimizer class
    fMinimizer = std::shared_ptr<ROOT::Math::Minimizer>(ROOT::Math::Factory::CreateMinimizer(minimType, "Migrad"));