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

.github/workflows/root-ci-config/buildconfig/alma9-modules_off.txt

@@ -1,4 +1,5 @@
 builtin_vdt=ON
+minuit1=OFF
 pythia8=ON
 runtime_cxxmodules=OFF
 use_gsl_cblas=ON

cmake/modules/RootBuildOptions.cmake

@@ -143,6 +143,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

@@ -493,6 +493,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

@@ -108,6 +108,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/testSumW2Error.cxx

@@ -49,55 +49,38 @@ TEST(SumW2Error, BatchMode)
 
    using namespace RooFit;
 
-   auto fit = [&](RooAbsData &data, bool sumw2, EvalBackend evalBackend, std::string const &minimizer,
-                  int printLevel = -1) {
+   auto fit = [&](RooAbsData &data, bool sumw2, EvalBackend evalBackend, int printLevel = -1) {
       params.assign(initialParams);
 
-      return std::unique_ptr<RooFitResult>{model.fitTo(data, Save(), SumW2Error(sumw2), Strategy(1), evalBackend,
-                                                       Minimizer(minimizer.c_str()), PrintLevel(printLevel))};
+      return std::unique_ptr<RooFitResult>{
+         model.fitTo(data, Save(), SumW2Error(sumw2), Strategy(1), evalBackend, PrintLevel(printLevel))};
    };
 
    auto scalar = EvalBackend::Legacy();
    auto batchMode = EvalBackend::Cpu();
 
    // Compare batch mode vs. scalar mode for non-SumW2 fits on UNWEIGHTED datasets
-   EXPECT_TRUE(fit(*dataSet, 0, scalar, "Minuit")->isIdentical(*fit(*dataSet, 0, batchMode, "Minuit")))
-      << " different results for Minuit fit to RooDataSet without SumW2Error correction.";
-   EXPECT_TRUE(fit(*dataHist, 0, scalar, "Minuit")->isIdentical(*fit(*dataHist, 0, batchMode, "Minuit")))
-      << " different results for Minuit fit to RooDataHist without SumW2Error correction.";
-   EXPECT_TRUE(fit(*dataSet, 0, scalar, "Minuit2")->isIdentical(*fit(*dataSet, 0, batchMode, "Minuit2")))
-      << " different results for Minuit2 fit to RooDataSet without SumW2Error correction.";
-   EXPECT_TRUE(fit(*dataHist, 0, scalar, "Minuit2")->isIdentical(*fit(*dataHist, 0, batchMode, "Minuit2")))
-      << " different results for Minuit2 fit to RooDataHist without SumW2Error correction.";
+   EXPECT_TRUE(fit(*dataSet, 0, scalar)->isIdentical(*fit(*dataSet, 0, batchMode)))
+      << " different results for fit to RooDataSet without SumW2Error correction.";
+   EXPECT_TRUE(fit(*dataHist, 0, scalar)->isIdentical(*fit(*dataHist, 0, batchMode)))
+      << " different results for fit to RooDataHist without SumW2Error correction.";
 
    // We can't compare the covariance matrix in these next cases, because it is
    // externally provided. Still, it's okay because the parameter values and
    // errors are compared, where the errors are inferred from the external
    // covariance matrix.
 
    // Compare batch mode vs. scalar mode for SumW2 fits on UNWEIGHTED datasets
-   EXPECT_TRUE(fit(*dataSet, 1, scalar, "Minuit")->isIdenticalNoCov(*fit(*dataSet, 1, batchMode, "Minuit")))
-      << " different results for Minuit fit to RooDataSet with SumW2Error correction.";
-   EXPECT_TRUE(fit(*dataHist, 1, scalar, "Minuit")->isIdenticalNoCov(*fit(*dataHist, 1, batchMode, "Minuit")))
-      << " different results for Minuit fit to RooDataHist with SumW2Error correction.";
-   EXPECT_TRUE(fit(*dataSet, 1, scalar, "Minuit2")->isIdenticalNoCov(*fit(*dataSet, 1, batchMode, "Minuit2")))
-      << " different results for Minuit2 fit to RooDataSet with SumW2Error correction.";
-   EXPECT_TRUE(fit(*dataHist, 1, scalar, "Minuit2")->isIdenticalNoCov(*fit(*dataHist, 1, batchMode, "Minuit2")))
-      << " different results for Minuit2 fit to RooDataHist with SumW2Error correction.";
+   EXPECT_TRUE(fit(*dataSet, 1, scalar)->isIdenticalNoCov(*fit(*dataSet, 1, batchMode)))
+      << " different results for fit to RooDataSet with SumW2Error correction.";
+   EXPECT_TRUE(fit(*dataHist, 1, scalar)->isIdenticalNoCov(*fit(*dataHist, 1, batchMode)))
+      << " different results for fit to RooDataHist with SumW2Error correction.";
 
    // Compare batch mode vs. scalar mode for SumW2 fits on WEIGHTED datasets
-   EXPECT_TRUE(
-      fit(dataSetWeighted, 1, scalar, "Minuit")->isIdenticalNoCov(*fit(dataSetWeighted, 1, batchMode, "Minuit")))
-      << " different results for Minuit fit to weighted RooDataSet with SumW2Error correction.";
-   EXPECT_TRUE(
-      fit(*dataHistWeighted, 1, scalar, "Minuit")->isIdenticalNoCov(*fit(*dataHistWeighted, 1, batchMode, "Minuit")))
-      << " different results for Minuit fit to weighted RooDataHist with SumW2Error correction.";
-   EXPECT_TRUE(
-      fit(dataSetWeighted, 1, scalar, "Minuit2")->isIdenticalNoCov(*fit(dataSetWeighted, 1, batchMode, "Minuit2")))
-      << " different results for Minuit2 fit to weighted RooDataSet with SumW2Error correction.";
-   EXPECT_TRUE(
-      fit(*dataHistWeighted, 1, scalar, "Minuit2")->isIdenticalNoCov(*fit(*dataHistWeighted, 1, batchMode, "Minuit2")))
-      << " different results for Minuit2 fit to weighted RooDataHist with SumW2Error correction.";
+   EXPECT_TRUE(fit(dataSetWeighted, 1, scalar)->isIdenticalNoCov(*fit(dataSetWeighted, 1, batchMode)))
+      << " different results for fit to weighted RooDataSet with SumW2Error correction.";
+   EXPECT_TRUE(fit(*dataHistWeighted, 1, scalar)->isIdenticalNoCov(*fit(*dataHistWeighted, 1, batchMode)))
+      << " different results for fit to weighted RooDataHist with SumW2Error correction.";
 }
 
 TEST(SumW2Error, ExtendedFit)
@@ -128,11 +111,8 @@ TEST(SumW2Error, ExtendedFit)
    auto *wFunc = ws.factory("w[1.5]");
 
    auto *w = dataNoWeights->addColumn(*wFunc);
-   RooDataSet data{dataNoWeights->GetName(),
-                   dataNoWeights->GetTitle(),
-                   *dataNoWeights->get(),
-                   RooFit::Import(*dataNoWeights),
-                   RooFit::WeightVar(w->GetName())};
+   RooDataSet data{dataNoWeights->GetName(), dataNoWeights->GetTitle(), *dataNoWeights->get(),
+                   RooFit::Import(*dataNoWeights), RooFit::WeightVar(w->GetName())};
    RooDataHist datahist{"datahist", "datahist", *data.get(), data};
 
    RooArgSet params;
@@ -143,9 +123,8 @@ TEST(SumW2Error, ExtendedFit)
 
    auto doFit = [&](RooFit::EvalBackend evalBackend, bool sumW2Error, const char *range) {
       params.assign(initialParams);
-      return std::unique_ptr<RooFitResult>{shp->fitTo(datahist, Extended(), Range(range), Save(),
-                                                      SumW2Error(sumW2Error), Strategy(1), PrintLevel(-1), evalBackend,
-                                                      Minimizer("Minuit2", "migrad"))};
+      return std::unique_ptr<RooFitResult>{shp->fitTo(
+         datahist, Extended(), Range(range), Save(), SumW2Error(sumW2Error), Strategy(1), PrintLevel(-1), evalBackend)};
    };
 
    // compare batch mode and scalar mode fit results for full range

roofit/roostats/CMakeLists.txt

@@ -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/roostats/src/LikelihoodInterval.cxx

@@ -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"));
 

roofit/roostats/test/CMakeLists.txt

@@ -11,14 +11,16 @@ if(mathmore)
 endif()
 
 configure_file(stressRooStats_ref.root stressRooStats_ref.root COPYONLY)
-if(roofit_legacy_eval_backend)
-  ROOT_ADD_TEST(test-stressroostats-legacy COMMAND stressRooStats -b legacy FAILREGEX "FAILED|Error in" LABELS longtest)
+if(minuit1)
+  if(roofit_legacy_eval_backend)
+    ROOT_ADD_TEST(test-stressroostats-legacy-minuit1 COMMAND stressRooStats -minim Minuit -b legacy FAILREGEX "FAILED|Error in" LABELS longtest)
+  endif()
+  ROOT_ADD_TEST(test-stressroostats-cpu-minuit1 COMMAND stressRooStats -minim Minuit -b cpu FAILREGEX "FAILED|Error in" LABELS longtest)
 endif()
-ROOT_ADD_TEST(test-stressroostats-cpu COMMAND stressRooStats -b cpu FAILREGEX "FAILED|Error in" LABELS longtest)
 if(cuda)
   ROOT_ADD_TEST(test-stressroostats-cuda COMMAND stressRooStats -b cuda FAILREGEX "FAILED|Error in" LABELS longtest RESOURCE_LOCK GPU)
 endif()
 if(roofit_legacy_eval_backend)
-  ROOT_ADD_TEST(test-stressroostats-legacy-minuit2 COMMAND stressRooStats -minim Minuit2 -b legacy FAILREGEX "FAILED|Error in" LABELS longtest)
+  ROOT_ADD_TEST(test-stressroostats-legacy COMMAND stressRooStats -b legacy FAILREGEX "FAILED|Error in" LABELS longtest)
 endif()
-ROOT_ADD_TEST(test-stressroostats-cpu-minuit2 COMMAND stressRooStats -minim Minuit2 -b cpu FAILREGEX "FAILED|Error in" LABELS longtest)
+ROOT_ADD_TEST(test-stressroostats-cpu COMMAND stressRooStats -b cpu FAILREGEX "FAILED|Error in" LABELS longtest)

roofit/roostats/test/stressRooStats.cxx

@@ -313,7 +313,7 @@ int main(int argc, const char *argv[])
 
    // string refFileName = "http://root.cern/files/stressRooStats_v534_ref.root" ;
    string refFileName = "stressRooStats_ref.root";
-   string minimizerName = "Minuit";
+   string minimizerName = "Minuit2";
 
    // Parse command line arguments
    for (int i = 1; i < argc; i++) {

tmva/tmva/CMakeLists.txt

@@ -9,6 +9,20 @@
 # @author Pere Mato, CERN
 ############################################################################
 
+if(minuit1)
+  set(TMVA_EXTRA_HEADERS
+    TMVA/MinuitFitter.h
+    TMVA/MinuitWrapper.h
+  )
+  set(TMVA_EXTRA_SOURCES
+    src/MinuitFitter.cxx
+    src/MinuitWrapper.cxx
+  )
+  set(TMVA_EXTRA_DEPENDENCIES
+    Minuit
+  )
+endif()
+
 if(NOT tmva-gpu)
   set(installoptions ${installoptions} FILTER "Cuda")
 endif()
@@ -99,8 +113,6 @@ ROOT_STANDARD_LIBRARY_PACKAGE(TMVA
     TMVA/MethodRuleFit.h
     TMVA/MethodSVM.h
     TMVA/MethodTMlpANN.h
-    TMVA/MinuitFitter.h
-    TMVA/MinuitWrapper.h
     TMVA/MisClassificationError.h
     TMVA/ModulekNN.h
     TMVA/Monitoring.h
@@ -288,8 +300,6 @@ ROOT_STANDARD_LIBRARY_PACKAGE(TMVA
     src/MethodRuleFit.cxx
     src/MethodSVM.cxx
     src/MethodTMlpANN.cxx
-    src/MinuitFitter.cxx
-    src/MinuitWrapper.cxx
     src/MisClassificationError.cxx
     src/ModulekNN.cxx
     src/MsgLogger.cxx
@@ -386,7 +396,6 @@ ROOT_STANDARD_LIBRARY_PACKAGE(TMVA
     Tree
     Hist
     Matrix
-    Minuit
     MLP
     MathCore
     Core
@@ -400,6 +409,10 @@ ROOT_STANDARD_LIBRARY_PACKAGE(TMVA
   ${EXTRA_DICT_OPTS}
 )
 
+if(minuit1)
+  target_compile_definitions(TMVA PRIVATE TMVA_MINUIT_1)
+endif()
+
 if(MSVC)
   target_compile_definitions(TMVA PRIVATE _USE_MATH_DEFINES)
 endif()

tmva/tmva/src/MethodCuts.cxx

@@ -91,7 +91,9 @@
 #include "TMVA/MCFitter.h"
 #include "TMVA/MethodBase.h"
 #include "TMVA/MethodFDA.h"
+#ifdef TMVA_MINUIT_1
 #include "TMVA/MinuitFitter.h"
+#endif
 #include "TMVA/MsgLogger.h"
 #include "TMVA/PDF.h"
 #include "TMVA/Results.h"
@@ -674,7 +676,11 @@ void  TMVA::MethodCuts::Train( void )
          fitter = new MCFitter     ( *this, TString::Format("%sFitter_MC",     GetName()), ranges, GetOptions() );
          break;
       case kUseMinuit:
+#ifdef TMVA_MINUIT_1
          fitter = new MinuitFitter ( *this, TString::Format("%sFitter_MINUIT", GetName()), ranges, GetOptions() );
+#else
+         Log() << kFATAL << "Can't use MINUIT fitter because ROOT was build without MINUIT 1" << Endl;
+#endif
          break;
       case kUseSimulatedAnnealing:
          fitter = new SimulatedAnnealingFitter( *this, TString::Format("%sFitter_SA", GetName()), ranges, GetOptions() );

tmva/tmva/src/MethodFDA.cxx

@@ -53,7 +53,9 @@ Can compute regression value for one dimensional output
 #include "TMVA/IMethod.h"
 #include "TMVA/MCFitter.h"
 #include "TMVA/MethodBase.h"
+#ifdef TMVA_MINUIT_1
 #include "TMVA/MinuitFitter.h"
+#endif
 #include "TMVA/MsgLogger.h"
 #include "TMVA/Timer.h"
 #include "TMVA/Tools.h"
@@ -300,8 +302,12 @@ void TMVA::MethodFDA::ProcessOptions()
    // create minimiser
    fConvergerFitter = (IFitterTarget*)this;
    if (fConverger == "MINUIT") {
+#ifdef TMVA_MINUIT_1
       fConvergerFitter = new MinuitFitter( *this, TString::Format("%s_Converger_Minuit", GetName()), fParRange, GetOptions() );
       SetOptions(dynamic_cast<Configurable*>(fConvergerFitter)->GetOptions());
+#else
+      Log() << kFATAL << "Can't use MINUIT converger because ROOT was build without MINUIT 1" << Endl;
+#endif
    }
 
    if(fFitMethod == "MC")
@@ -310,9 +316,13 @@ void TMVA::MethodFDA::ProcessOptions()
       fFitter = new GeneticFitter( *fConvergerFitter, TString::Format("%s_Fitter_GA", GetName()), fParRange, GetOptions() );
    else if (fFitMethod == "SA")
       fFitter = new SimulatedAnnealingFitter( *fConvergerFitter, TString::Format("%s_Fitter_SA", GetName()), fParRange, GetOptions() );
-   else if (fFitMethod == "MINUIT")
+   else if (fFitMethod == "MINUIT") {
+#ifdef TMVA_MINUIT_1
       fFitter = new MinuitFitter( *fConvergerFitter, TString::Format("%s_Fitter_Minuit", GetName()), fParRange, GetOptions() );
-   else {
+#else
+      Log() << kFATAL << "Can't use MINUIT fitter because ROOT was build without MINUIT 1" << Endl;
+#endif
+   } else {
       Log() << kFATAL << "<Train> Do not understand fit method:" << fFitMethod << Endl;
    }
 

tmva/tmva/src/MethodMLP.cxx

@@ -65,7 +65,9 @@ Multilayer Perceptron class built off of MethodANNBase
 
 #include "TH1.h"
 #include "TString.h"
+#ifdef MethodMLP_UseMinuit__
 #include "TFitter.h"
+#endif
 #include "TMatrixD.h"
 #include "TMath.h"
 

tmva/tmva/src/OptimizeConfigParameters.cxx

@@ -38,7 +38,9 @@
 #include "TMVA/MethodBase.h"
 #include "TMVA/MethodFDA.h"
 #include "TMVA/MsgLogger.h"
+#ifdef TMVA_MINUIT_1
 #include "TMVA/MinuitFitter.h"
+#endif
 #include "TMVA/PDF.h"
 #include "TMVA/Tools.h"
 #include "TMVA/Types.h"
@@ -263,9 +265,13 @@ void TMVA::OptimizeConfigParameters::optimizeFit()
       TString opt="FitStrategy=0:UseImprove=False:UseMinos=False:Tolerance=100";
       if (!TMVA::gConfig().IsSilent() ) opt += TString(":PrintLevel=0");
 
+#ifdef TMVA_MINUIT_1
       fitter = new MinuitFitter(  *this,
                                   "FitterMinuit_BDTOptimize",
                                   ranges, opt );
+#else
+      Log() << kFATAL << "Can't use MINUIT optimizer because ROOT was build without MINUIT 1" << Endl;
+#endif
    }else if ( fOptimizationFitType == "FitGA"  ) {
       TString opt="PopSize=20:Steps=30:Cycles=3:ConvCrit=0.01:SaveBestCycle=5";
       fitter = new GeneticFitter( *this,