From eeb581a40853066617349d32b72a13102df8f992 Mon Sep 17 00:00:00 2001
From: Gennady Pospelov <g.pospelov@fz-juelich.de>
Date: Tue, 17 Jan 2017 15:37:50 +0100
Subject: [PATCH] FitParameterSet now sets names of fit parameters
 automatically.

---
 Core/Fitting/FitParameterLinked.cpp           | 14 -------
 Core/Fitting/FitParameterLinked.h             |  6 ---
 Core/Fitting/FitSuite.h                       |  2 +-
 Core/Fitting/FitSuiteImpl.cpp                 |  4 +-
 Core/Fitting/FitSuiteImpl.h                   |  2 +-
 .../FitSpheresInHexLattice.py                 |  2 +
 Fit/Parameters/FitParameter.cpp               |  2 +-
 Fit/Parameters/FitParameterSet.cpp            | 20 +++++++---
 Fit/Parameters/FitParameterSet.h              |  1 +
 Fit/RootAdapter/MinimizerResultsHelper.cpp    |  7 +---
 Tests/Functional/PyFit/fitsuite_api.py        | 22 +++++-----
 Tests/UnitTests/Fit/0/FitParameterSetTest.h   | 40 ++++++++++---------
 Tests/UnitTests/Fit/0/FitSuiteTest.h          | 18 ++++-----
 13 files changed, 65 insertions(+), 75 deletions(-)

diff --git a/Core/Fitting/FitParameterLinked.cpp b/Core/Fitting/FitParameterLinked.cpp
index c363cc80063..565e2a0bc3b 100644
--- a/Core/Fitting/FitParameterLinked.cpp
+++ b/Core/Fitting/FitParameterLinked.cpp
@@ -98,17 +98,3 @@ FitParameterLinked::FitParameterLinked(const FitParameterLinked& other)
         m_pool_parameters.push_back(par->clone());
     m_patterns = other.m_patterns;
 }
-
-void FitParameterLinked::print(std::ostream& ostr) const
-{
-    ostr << FitParameter::toString();
-    ostr << "FitParameterLinked '" << name() << "'"
-         << " value:" << value() << " collsize:" << m_pool_parameters.size();
-    //    if(m_parametercoll.size() ) {
-    //        ostr << " addresses: ";
-    //        for(auto it=m_parametercoll.begin(); it!=m_parametercoll.end(); it++) {
-    //            parameter_t par = (*it);
-    //            ostr << par << " ";
-    //        }
-    //    }
-}
diff --git a/Core/Fitting/FitParameterLinked.h b/Core/Fitting/FitParameterLinked.h
index cfb0b9a6040..198e27aa708 100644
--- a/Core/Fitting/FitParameterLinked.h
+++ b/Core/Fitting/FitParameterLinked.h
@@ -43,16 +43,10 @@ class BA_CORE_API_ FitParameterLinked : public FitParameter
 
     void addMatchedParameters(const ParameterPool& pool);
 
-    friend std::ostream& operator<<(std::ostream& ostr, const FitParameterLinked& m) {
-        m.print(ostr); return ostr; }
-
     std::vector<std::string> matchedParameterNames() const;
 
  private:
     FitParameterLinked(const FitParameterLinked& other);
-
-    void print(std::ostream& ostr) const;
-
     std::vector<RealParameter*> m_pool_parameters; //!< linked parameters from pools
     std::vector<std::string> m_patterns; //!< list of patterns to match from pool
 };
diff --git a/Core/Fitting/FitSuite.h b/Core/Fitting/FitSuite.h
index aadf4200f29..e82a86e9e36 100644
--- a/Core/Fitting/FitSuite.h
+++ b/Core/Fitting/FitSuite.h
@@ -64,7 +64,7 @@ public:
                                   double weight=1);
 
     //! Adds fit parameter
-    //! @param name The name of fit parameter
+    //! @param name The name of sample parameter(s) to fit (may contain wildcards).
     //! @param value Parameter's starting value
     //! @param limits Limits attribute
     //! @param step Initial parameter's step (some minimizers don't use it)
diff --git a/Core/Fitting/FitSuiteImpl.cpp b/Core/Fitting/FitSuiteImpl.cpp
index cd8526539f2..f0dc82570da 100644
--- a/Core/Fitting/FitSuiteImpl.cpp
+++ b/Core/Fitting/FitSuiteImpl.cpp
@@ -58,13 +58,13 @@ FitObject* FitSuiteImpl::addSimulationAndRealData(const GISASSimulation& simulat
 }
 
 //! Adds fit parameter, step is calculated from initial parameter value
-FitParameterLinked* FitSuiteImpl::addFitParameter(const std::string& name, double value,
+FitParameterLinked* FitSuiteImpl::addFitParameter(const std::string& pattern, double value,
                                   const AttLimits& limits, double step)
 {
     if(step <=0.0)
         step = value * getOptions().stepFactor();
 
-    FitParameterLinked* result = new FitParameterLinked(name, value, limits, step);
+    FitParameterLinked* result = new FitParameterLinked(pattern, value, limits, step);
     m_kernel->fitParameters()->addFitParameter(result);
     return result;
 }
diff --git a/Core/Fitting/FitSuiteImpl.h b/Core/Fitting/FitSuiteImpl.h
index 9b84939e706..cc0ee1df50c 100644
--- a/Core/Fitting/FitSuiteImpl.h
+++ b/Core/Fitting/FitSuiteImpl.h
@@ -52,7 +52,7 @@ class BA_CORE_API_ FitSuiteImpl
                                   double weight);
 
     //! Adds fit parameter
-    FitParameterLinked* addFitParameter(const std::string& name, double value,
+    FitParameterLinked* addFitParameter(const std::string& pattern, double value,
                                         const AttLimits& limits, double step = 0.0);
 
     //! Adds fit strategy
diff --git a/Examples/python/fitting/ex03_FitSpheresInHexLattice/FitSpheresInHexLattice.py b/Examples/python/fitting/ex03_FitSpheresInHexLattice/FitSpheresInHexLattice.py
index 911f84daec5..08c0824ec80 100644
--- a/Examples/python/fitting/ex03_FitSpheresInHexLattice/FitSpheresInHexLattice.py
+++ b/Examples/python/fitting/ex03_FitSpheresInHexLattice/FitSpheresInHexLattice.py
@@ -1,5 +1,7 @@
 """
 Two parameter fit of spheres in a hex lattice.
+See FitSpheresInHexLattice_builder.py example which performs same fitting task
+using advanced sample construction techniques.
 """
 
 from matplotlib import pyplot as plt
diff --git a/Fit/Parameters/FitParameter.cpp b/Fit/Parameters/FitParameter.cpp
index 9dd34746db5..c74bdf517b8 100644
--- a/Fit/Parameters/FitParameter.cpp
+++ b/Fit/Parameters/FitParameter.cpp
@@ -119,7 +119,7 @@ std::string FitParameter::toString() const
 {
     std::ostringstream ostr;
 
-    const int max_length_of_name(40);
+    const int max_length_of_name(10);
     std::string adjusted_name = m_name;
     adjusted_name.resize(max_length_of_name,' ');
     ostr << adjusted_name << std::scientific << std::setprecision(8) << m_value << "  ";
diff --git a/Fit/Parameters/FitParameterSet.cpp b/Fit/Parameters/FitParameterSet.cpp
index e03a188e87d..98018f5922f 100644
--- a/Fit/Parameters/FitParameterSet.cpp
+++ b/Fit/Parameters/FitParameterSet.cpp
@@ -68,14 +68,16 @@ FitParameterSet::const_iterator FitParameterSet::end() const
 
 void FitParameterSet::addFitParameter(FitParameter* par)
 {
-    if(isExistingName(par->name()))
-        throw std::runtime_error("FitParameterSet::addFitParameter() -> Error. Parameter with "
-                                 "the name '"+par->name()+"' already exist.");
-
     for (auto fitPar: m_parameters)
         if(fitPar == par)
             throw std::runtime_error("FitParameterSet::addFitParameter() -> Error. Attempt to add "
                                      "same fit parameter twice.");
+
+    par->setName(suggestParameterName());
+    if(isExistingName(par->name()))
+        throw std::runtime_error("FitParameterSet::addFitParameter() -> Error. Parameter with "
+                                 "the name '"+par->name()+"' already exist.");
+
     m_parameters.push_back(par);
 }
 
@@ -220,9 +222,8 @@ void FitParameterSet::setFixed(const std::vector<std::string>& pars, bool is_fix
 std::string FitParameterSet::parametersToString() const
 {
     std::ostringstream result;
-    int npar = 0;
     for (auto par: m_parameters)
-        result << "   # "<< npar++ << " " << par->toString() << "\n";
+        result << "   # " << par->toString() << "\n";
     return result.str();
 }
 
@@ -244,6 +245,13 @@ bool FitParameterSet::isExistingName(const std::string& name) const
     return false;
 }
 
+//! Returns parameter name basing on fixed prefix 'par' and number of parameters.
+
+std::string FitParameterSet::suggestParameterName() const
+{
+    return std::string("par")+std::to_string(m_parameters.size());
+}
+
 //! Checks if index corresponds with the number of fit parameters.
 
 size_t FitParameterSet::check_index(size_t index) const
diff --git a/Fit/Parameters/FitParameterSet.h b/Fit/Parameters/FitParameterSet.h
index 51491af6d39..bf06d84cfba 100644
--- a/Fit/Parameters/FitParameterSet.h
+++ b/Fit/Parameters/FitParameterSet.h
@@ -79,6 +79,7 @@ class BA_CORE_API_ FitParameterSet
     bool isExistingName(const std::string& name) const;
 
 private:
+    std::string suggestParameterName() const;
     size_t check_index(size_t index) const;
     void check_array_size(const std::vector<double>& values) const;
     container_t m_parameters; //!< collection of fit parameters
diff --git a/Fit/RootAdapter/MinimizerResultsHelper.cpp b/Fit/RootAdapter/MinimizerResultsHelper.cpp
index fcd49dfe73d..027a0d700f2 100644
--- a/Fit/RootAdapter/MinimizerResultsHelper.cpp
+++ b/Fit/RootAdapter/MinimizerResultsHelper.cpp
@@ -55,18 +55,15 @@ std::string MinimizerResultsHelper::reportParameters(const FitParameterSet* para
 
     result << MinimizerUtils::sectionString("FitParameters");
 
-    result << "Npar Name        StartValue  Limits           FitValue  Error" << std::endl;
+    result << "Name       StartValue  Limits           FitValue  Error" << std::endl;
 
-    int npar(0);
     for(const FitParameter* par : *parameters) {
-        result << boost::format("#%-2d  %-10s  %-6.4f      %-15s  %-6.4f    %5.4f \n")
-                  % npar
+        result << boost::format("# %-8s %-7.4f     %-15s  %-6.4f    %5.4f \n")
                   % par->name()
                   % par->startValue()
                   % par->limits().toString()
                   % par->value()
                   % par->error();
-        ++npar;
     }
 
     FitParameterSet::corr_matrix_t matrix = parameters->correlationMatrix();
diff --git a/Tests/Functional/PyFit/fitsuite_api.py b/Tests/Functional/PyFit/fitsuite_api.py
index e1e6853b778..5f46cb6b607 100644
--- a/Tests/Functional/PyFit/fitsuite_api.py
+++ b/Tests/Functional/PyFit/fitsuite_api.py
@@ -15,7 +15,7 @@ class FitSuiteAPITest(unittest.TestCase):
         Testing of python iterator over defined fit parameters.
         """
         fitSuite = ba.FitSuite()
-        names = ["par1", "par2", "par3"]
+        names = ["par0", "par1", "par2"]
         values = [1.0, 2.0, 3.0]
         for name, value in zip(names, values):
             fitSuite.addFitParameter(name, value)
@@ -30,20 +30,20 @@ class FitSuiteAPITest(unittest.TestCase):
 
     def test_addFitParameter(self):
         fitSuite = ba.FitSuite()
-        fitSuite.addFitParameter("par1", 1.0)
-        fitSuite.addFitParameter("par2", 2.0, ba.AttLimits.limited(10.0, 20.0), 0.02)
-        fitSuite.addFitParameter("par3", 3.0).setLowerLimited(30.0).setStep(0.03)
-        fitSuite.addFitParameter("par4", 4.0).setStep(0.04).setUpperLimited(40.0)
-        fitSuite.addFitParameter("par5", 5.0).setFixed()
+        fitSuite.addFitParameter("pattern0", 1.0)
+        fitSuite.addFitParameter("pattern1", 2.0, ba.AttLimits.limited(10.0, 20.0), 0.02)
+        fitSuite.addFitParameter("pattern2", 3.0).setLowerLimited(30.0).setStep(0.03)
+        fitSuite.addFitParameter("pattern3", 4.0).setStep(0.04).setUpperLimited(40.0)
+        fitSuite.addFitParameter("pattern4", 5.0).setFixed()
 
         par = fitSuite.fitParameters()[0]
-        self.assertEqual(par.name(), "par1")
+        self.assertEqual(par.name(), "par0")
         self.assertEqual(par.value(), 1.0)
         self.assertEqual(par.step(), 0.01)  # default step is 1% in FitSuite
         self.assertTrue(par.limits().isLimitless())
 
         par = fitSuite.fitParameters()[1]
-        self.assertEqual(par.name(), "par2")
+        self.assertEqual(par.name(), "par1")
         self.assertEqual(par.value(), 2.0)
         self.assertEqual(par.step(), 0.02)
         self.assertTrue(par.limits().isLimited())
@@ -51,21 +51,21 @@ class FitSuiteAPITest(unittest.TestCase):
         self.assertEqual(par.limits().upperLimit(), 20.0)
 
         par = fitSuite.fitParameters()[2]
-        self.assertEqual(par.name(), "par3")
+        self.assertEqual(par.name(), "par2")
         self.assertEqual(par.value(), 3.0)
         self.assertEqual(par.step(), 0.03)
         self.assertTrue(par.limits().isLowerLimited())
         self.assertEqual(par.limits().lowerLimit(), 30.0)
 
         par = fitSuite.fitParameters()[3]
-        self.assertEqual(par.name(), "par4")
+        self.assertEqual(par.name(), "par3")
         self.assertEqual(par.value(), 4.0)
         self.assertEqual(par.step(), 0.04)
         self.assertTrue(par.limits().isUpperLimited())
         self.assertEqual(par.limits().upperLimit(), 40.0)
 
         par = fitSuite.fitParameters()[4]
-        self.assertEqual(par.name(), "par5")
+        self.assertEqual(par.name(), "par4")
         self.assertEqual(par.value(), 5.0)
         self.assertTrue(par.limits().isFixed())
 
diff --git a/Tests/UnitTests/Fit/0/FitParameterSetTest.h b/Tests/UnitTests/Fit/0/FitParameterSetTest.h
index 300782038ae..4f4df8ac430 100644
--- a/Tests/UnitTests/Fit/0/FitParameterSetTest.h
+++ b/Tests/UnitTests/Fit/0/FitParameterSetTest.h
@@ -13,16 +13,18 @@ TEST_F(FitParameterSetTest, addFitParameter)
 {
     FitParameterSet parameters;
 
-    FitParameter *par = new FitParameter("par1", 1.0);
+    FitParameter *par = new FitParameter("pattern1", 1.0);
     parameters.addFitParameter(par);
     EXPECT_EQ(parameters.size(), 1u);
+    EXPECT_EQ(par->name(), "par0"); // names are fixed
 
     // attempt to add same fit parameter twice
     EXPECT_THROW(parameters.addFitParameter(par), std::runtime_error);
 
-    // attempt to add fit parameter with the same name
-    std::unique_ptr<FitParameter> par2(new FitParameter("par1", 2.0));
-    EXPECT_THROW(parameters.addFitParameter(par2.get()), std::runtime_error);
+    FitParameter *par2 = new FitParameter("pattern2", 1.0);
+    parameters.addFitParameter(par2);
+    EXPECT_EQ(parameters.size(), 2u);
+    EXPECT_EQ(par2->name(), "par1");
 
     parameters.clear();
     EXPECT_EQ(parameters.size(), 0u);
@@ -32,15 +34,15 @@ TEST_F(FitParameterSetTest, getFitParameter)
 {
     FitParameterSet parameters;
 
-    FitParameter *par1 = new FitParameter("par1", 1.0);
+    FitParameter *par1 = new FitParameter("pattern1", 1.0);
     parameters.addFitParameter(par1);
-    FitParameter *par2 = new FitParameter("par2", 1.0);
+    FitParameter *par2 = new FitParameter("pattern2", 1.0);
     parameters.addFitParameter(par2);
 
-    EXPECT_EQ(parameters.fitParameter("par1"), par1);
-    EXPECT_EQ(parameters.fitParameter("par2"), par2);
-    EXPECT_EQ(parameters["par1"], par1);
-    EXPECT_EQ(parameters["par2"], par2);
+    EXPECT_EQ(parameters.fitParameter("par0"), par1);
+    EXPECT_EQ(parameters.fitParameter("par1"), par2);
+    EXPECT_EQ(parameters["par0"], par1);
+    EXPECT_EQ(parameters["par1"], par2);
     EXPECT_EQ(parameters[0], par1);
     EXPECT_EQ(parameters[1], par2);
 
@@ -52,9 +54,9 @@ TEST_F(FitParameterSetTest, getFitParameter)
 TEST_F(FitParameterSetTest, parameterValues)
 {
     FitParameterSet parameters;
-    parameters.addFitParameter(new FitParameter("par1", 1.0));
-    parameters.addFitParameter(new FitParameter("par2", 2.0));
-    parameters.addFitParameter(new FitParameter("par3", 3.0));
+    parameters.addFitParameter(new FitParameter("pattern1", 1.0));
+    parameters.addFitParameter(new FitParameter("pattern2", 2.0));
+    parameters.addFitParameter(new FitParameter("pattern3", 3.0));
     std::vector<double> values{1.0, 2.0, 3.0};
     EXPECT_EQ(parameters.values(), values);
 
@@ -75,8 +77,8 @@ TEST_F(FitParameterSetTest, parameterValues)
 TEST_F(FitParameterSetTest, parameterErrors)
 {
     FitParameterSet parameters;
-    FitParameter *par1 = new FitParameter("par1", 1.0, AttLimits::limitless(), 0.01);
-    FitParameter *par2 = new FitParameter("par2", 1.0, AttLimits::limitless(), 0.01);
+    FitParameter *par1 = new FitParameter("pattern1", 1.0, AttLimits::limitless(), 0.01);
+    FitParameter *par2 = new FitParameter("pattern2", 1.0, AttLimits::limitless(), 0.01);
 
     parameters.addFitParameter(par1);
     parameters.addFitParameter(par2);
@@ -94,9 +96,9 @@ TEST_F(FitParameterSetTest, parameterErrors)
 TEST_F(FitParameterSetTest, fixRelease)
 {
     FitParameterSet parameters;
-    FitParameter *par1 = new FitParameter("par1", 1.0, AttLimits::limitless(), 0.01);
-    FitParameter *par2 = new FitParameter("par2", 1.0, AttLimits::limitless(), 0.01);
-    FitParameter *par3 = new FitParameter("par3", 1.0, AttLimits::limitless(), 0.01);
+    FitParameter *par1 = new FitParameter("pattern1", 1.0, AttLimits::limitless(), 0.01);
+    FitParameter *par2 = new FitParameter("pattern2", 1.0, AttLimits::limitless(), 0.01);
+    FitParameter *par3 = new FitParameter("pattern3", 1.0, AttLimits::limitless(), 0.01);
 
     parameters.addFitParameter(par1);
     parameters.addFitParameter(par2);
@@ -115,7 +117,7 @@ TEST_F(FitParameterSetTest, fixRelease)
     parameters.releaseAll();
     EXPECT_EQ(parameters.freeFitParameterCount(), 3u);
 
-    std::vector<std::string> names_to_fix={"par1", "par3"};
+    std::vector<std::string> names_to_fix={"par0", "par2"};
     parameters.setFixed(names_to_fix, true);
     EXPECT_EQ(parameters.freeFitParameterCount(), 1u);
     EXPECT_TRUE(par1->limits().isFixed());
diff --git a/Tests/UnitTests/Fit/0/FitSuiteTest.h b/Tests/UnitTests/Fit/0/FitSuiteTest.h
index b633a77246d..6798c79842c 100644
--- a/Tests/UnitTests/Fit/0/FitSuiteTest.h
+++ b/Tests/UnitTests/Fit/0/FitSuiteTest.h
@@ -14,16 +14,15 @@ TEST_F(FitSuiteTest, addFitParameter)
 {
     std::unique_ptr<FitSuite> fitSuite(new FitSuite);
 
-    fitSuite->addFitParameter("par1", 1.0);
-    FitParameter *par = fitSuite->fitParameters()->fitParameter("par1");
-    EXPECT_EQ("par1", par->name());
+    FitParameter *par = fitSuite->addFitParameter("pattern1", 1.0);
+    EXPECT_EQ("par0", par->name());
     EXPECT_EQ(1.0, par->value());
     EXPECT_EQ(0.0, par->error());
     EXPECT_EQ(0.01, par->step()); // default step invented by FitSuite
     EXPECT_TRUE(par->limits().isLimitless());
 
-    par = fitSuite->addFitParameter("par2", 2.0, AttLimits::limited(10.0, 20.0), 0.02);
-    EXPECT_EQ("par2", par->name());
+    par = fitSuite->addFitParameter("pattern2", 2.0, AttLimits::limited(10.0, 20.0), 0.02);
+    EXPECT_EQ("par1", par->name());
     EXPECT_EQ(2.0, par->value());
     EXPECT_EQ(0.0, par->error());
     EXPECT_EQ(0.02, par->step());
@@ -31,16 +30,17 @@ TEST_F(FitSuiteTest, addFitParameter)
     EXPECT_EQ(10.0, par->limits().lowerLimit());
     EXPECT_EQ(20.0, par->limits().upperLimit());
 
-    FitParameter &par2 = fitSuite->addFitParameter("par3", 3.0)->setStep(0.03).setLowerLimited(30.0);
-    EXPECT_EQ("par3", par2.name());
+    FitParameter &par2 = fitSuite->addFitParameter("pattern3", 3.0)->
+            setStep(0.03).setLowerLimited(30.0);
+    EXPECT_EQ("par2", par2.name());
     EXPECT_EQ(3.0, par2.value());
     EXPECT_EQ(0.0, par2.error());
     EXPECT_EQ(0.03, par2.step());
     EXPECT_TRUE(par2.limits().isLowerLimited());
     EXPECT_EQ(30.0, par2.limits().lowerLimit());
 
-    FitParameter &par3 = fitSuite->addFitParameter("par4", 4.0)->setFixed();
-    EXPECT_EQ("par4", par3.name());
+    FitParameter &par3 = fitSuite->addFitParameter("pattern4", 4.0)->setFixed();
+    EXPECT_EQ("par3", par3.name());
     EXPECT_EQ(4.0, par3.value());
     EXPECT_TRUE(par3.limits().isFixed());
 }
-- 
GitLab