Revert "GModels access operator now returns reference instead of pointer."

This reverts commit 813252eb589e317fd139d0cdde4f20bdd7959ef2.

Revert "GModels access operator now returns reference instead of pointer."
This reverts commit 813252eb589e317fd139d0cdde4f20bdd7959ef2.
Jürgen Knödlseder
1 parent 813252eb
Showing 12 changed files with 119 additions and 89 deletions
NEWS
include/GModels.hpp
inst/com/test/test_COM.cpp
inst/cta/test/test_CTA.cpp
inst/lat/test/test_LAT.cpp
inst/mwl/test/test_MWL.cpp
pyext/GModels.i
src/model/GModels.cpp
src/obs/GObservation.cpp
test/test_GModel.cpp
test/test_GObservation.cpp
test/test_GOptimizer.cpp
@@ -36,9 +36,6 @@ GOptimizer &amp; GOptimizerLM
   The optimization operators have been removed and replaced by the
   optimize method.
-GModels
-  The model access operators now return the model by reference
-  instead of returning a pointer.
 3. Configuration
 -----------------
@@ -72,10 +72,10 @@ public:
     // Operators
     GModels&      operator=(const GModels& models);
-    GModel&       operator[](const int& index);
-    const GModel& operator[](const int& index) const;
-    GModel&       operator[](const std::string& name);
-    const GModel& operator[](const std::string& name) const;
+    GModel*       operator[](const int& index);
+    const GModel* operator[](const int& index) const;
+    GModel*       operator[](const std::string& name);
+    const GModel* operator[](const std::string& name) const;
     // Methods
     void          clear(void);
@@ -605,9 +605,9 @@ void TestGCOMOptimize::test_binned_optimizer(void)
         obs.optimize(opt);
         test_try_success();
         for (int i = 0, j = 0; i < obs.models().size(); ++i) {
-            const GModel& model = obs.models()[i];
-            for (int k = 0; k < model.size(); ++k) {
-                GModelPar par = model[k];
+            const GModel* model = obs.models()[i];
+            for (int k = 0; k < model->size(); ++k) {
+                GModelPar par  = (*model)[k];
                 std::string msg = "Verify optimization result for " + par.print();
                 test_value(par.real_value(), fit_results[j++], 5.0e-5, msg);
                 test_value(par.real_error(), fit_results[j++], 5.0e-5, msg);
@@ -525,9 +525,9 @@ void TestGCTAOptimize::test_unbinned_optimizer(void)
         obs.optimize(opt);
         test_try_success();
         for (int i = 0, j = 0; i < obs.models().size(); ++i) {
-            const GModel& model = obs.models()[i];
-            for (int k = 0; k < model.size(); ++k) {
-                GModelPar par = model[k];
+            const GModel* model = obs.models()[i];
+            for (int k = 0; k < model->size(); ++k) {
+                GModelPar par  = (*model)[k];
                 std::string msg = "Verify optimization result for " + par.print();
                 test_value(par.real_value(), fit_results[j++], 5.0e-5, msg);
                 test_value(par.real_error(), fit_results[j++], 5.0e-5, msg);
@@ -587,9 +587,9 @@ void TestGCTAOptimize::test_binned_optimizer(void)
         obs.optimize(opt);
         test_try_success();
         for (int i = 0, j = 0; i < obs.models().size(); ++i) {
-            const GModel& model = obs.models()[i];
-            for (int k = 0; k < model.size(); ++k) {
-                GModelPar par = model[k];
+            const GModel* model = obs.models()[i];
+            for (int k = 0; k < model->size(); ++k) {
+                GModelPar par  = (*model)[k];
                 std::string msg = "Verify optimization result for " + par.print();
                 test_value(par.real_value(), fit_results[j++], 5.0e-5, msg);
                 test_value(par.real_error(), fit_results[j++], 5.0e-5, msg);
@@ -663,9 +663,9 @@ void TestGLATOptimize::test_one_binned_optimizer(const std::string&amp; datadir,
         obs.optimize(opt);
         test_try_success();
         for (int i = 0, j = 0; i < obs.models().size(); ++i) {
-            const GModel& model = obs.models()[i];
-            for (int k = 0; k < model.size(); ++k) {
-                GModelPar par = model[k];
+            const GModel* model = obs.models()[i];
+            for (int k = 0; k < model->size(); ++k) {
+                GModelPar par  = (*model)[k];
                 std::string msg = "Verify optimization result for " + par.print();
                 test_value(par.real_value(), fit_results[j++], 5.0e-5, msg);
                 test_value(par.real_error(), fit_results[j++], 5.0e-5, msg);
@@ -221,9 +221,9 @@ void TestGMWL::test_optimizer(void)
         obs.optimize(opt);
         test_try_success();
         for (int i = 0, j = 0; i < obs.models().size(); ++i) {
-            const GModel& model = obs.models()[i];
-            for (int k = 0; k < model.size(); ++k) {
-                GModelPar par = model[k];
+            const GModel* model = obs.models()[i];
+            for (int k = 0; k < model->size(); ++k) {
+                GModelPar par = (*model)[k];
                 std::string msg = "Verify optimization result for " + par.print();
                 test_value(par.real_value(), fit_results[j++], 1.0e-4, msg);
                 test_value(par.real_error(), fit_results[j++], 1.0e-3, msg);
@@ -32,7 +32,7 @@
 %}
 /* __ Typemaps ___________________________________________________________ */
-%typemap(out) GModel& {
+%typemap(out) GModel* {
     if (dynamic_cast<GModelSky*>($1) != NULL) {
         $result = SWIG_NewPointerObj(SWIG_as_voidptr($1), SWIGTYPE_p_GModelSky, 0 |  0 );
     }
@@ -88,7 +88,7 @@ public:
     char *__str__() {
         return tochar(self->print());
     }
-    GModel& __getitem__(const int& index) {
+    GModel* __getitem__(const int& index) {
         if (index >= 0 && index < self->size()) {
             return (*self)[index];
         }
@@ -97,7 +97,7 @@ public:
                                            0, self->size()-1);
         }
     }
-    GModel& __getitem__(const std::string& name) {
+    GModel* __getitem__(const std::string& name) {
         return (*self)[name];
     }
     void __setitem__(const int& index, const GModel& val) {
@@ -162,16 +162,17 @@ GModels&amp; GModels::operator= (const GModels&amp; models)
 /***********************************************************************//**
- * @brief Return model
+ * @brief Return pointer to model
  *
  * @param[in] index Model index (0,...,size()-1).
  *
  * @exception GException::out_of_range
  *            Model index is out of range.
  *
- * Returns the model with the specified @p index.
+ * Returns a pointer to the model with the specified @p index.
+ * The return value will never be NULL.
  ***************************************************************************/
-GModel& GModels::operator[](const int& index)
+GModel* GModels::operator[](const int& index)
 {
     // Compile option: raise exception if index is out of range
     #if defined(G_RANGE_CHECK)
@@ -181,21 +182,22 @@ GModel&amp; GModels::operator[](const int&amp; index)
     #endif
     // Return pointer
-    return *(m_models[index]);
+    return m_models[index];
 }
 /***********************************************************************//**
- * @brief Return model (const version)
+ * @brief Return pointer to model (const version)
  *
  * @param[in] index Model index (0,...,size()-1).
  *
  * @exception GException::out_of_range
  *            Model index is out of range.
  *
- * Returns the model with the specified @p index.
+ * Returns a pointer to the model with the specified @p index.
+ * The return value will never be NULL.
  ***************************************************************************/
-const GModel& GModels::operator[](const int& index) const
+const GModel* GModels::operator[](const int& index) const
 {
     // Compile option: raise exception if index is out of range
     #if defined(G_RANGE_CHECK)
@@ -205,21 +207,22 @@ const GModel&amp; GModels::operator[](const int&amp; index) const
     #endif
     // Return pointer
-    return *(m_models[index]);
+    return m_models[index];
 }
 /***********************************************************************//**
- * @brief Return model
+ * @brief Return pointer to model
  *
  * @param[in] name Model name.
  *
  * @exception GException::model_not_found
  *            Model with specified name not found in container.
  *
- * Returns the model with the specified @p name.
+ * Returns a pointer to the model with the specified @p name.
+ * The return value will never be NULL.
  ***************************************************************************/
-GModel& GModels::operator[](const std::string& name)
+GModel* GModels::operator[](const std::string& name)
 {
     // Get parameter index
     int index = get_index(name);
@@ -230,21 +233,22 @@ GModel&amp; GModels::operator[](const std::string&amp; name)
     }
     // Return pointer
-    return *(m_models[index]);
+    return m_models[index];
 }
 /***********************************************************************//**
- * @brief Return model (const version)
+ * @brief Return pointer to model (const version)
  *
  * @param[in] name Model name.
  *
  * @exception GException::model_not_found
  *            Model with specified name not found in container.
  *
- * Returns the model with the specified @p name.
+ * Returns a pointer to the model with the specified @p name.
+ * The return value will never be NULL.
  ***************************************************************************/
-const GModel& GModels::operator[](const std::string& name) const
+const GModel* GModels::operator[](const std::string& name) const
 {
     // Get parameter index
     int index = get_index(name);
@@ -255,7 +259,7 @@ const GModel&amp; GModels::operator[](const std::string&amp; name) const
     }
     // Return pointer
-    return *(m_models[index]);
+    return m_models[index];
 }
@@ -308,8 +312,13 @@ GModels* GModels::clone(void) const
  ***************************************************************************/
 void GModels::append(const GModel& model)
 {
-    // Append model to the container
-    m_models.push_back(model.clone());
+    // Make deep copy of model
+    GModel* model_copy = model.clone();
+
+    // If copy is valid, append it to the container
+    if (model_copy != NULL) {
+        m_models.push_back(model_copy);
+    }
     // Set parameter pointers
     set_pointers();
@@ -340,8 +349,13 @@ void GModels::insert(const int&amp; index, const GModel&amp; model)
     }
     #endif
-    // Insert model into the container
-    m_models.insert(m_models.begin()+index, model.clone());
+    // Make deep copy of model
+    GModel* model_copy = model.clone();
+
+    // If copy is valid, insert it to the container
+    if (model_copy != NULL) {
+        m_models.insert(m_models.begin()+index, model_copy);
+    }
     // Set parameter pointers
     set_pointers();
@@ -373,8 +387,13 @@ void GModels::insert(const std::string&amp; name, const GModel&amp; model)
         throw GException::model_not_found(G_INSERT2, name);
     }
-    // Insert model into the container
-    m_models.insert(m_models.begin()+index, model.clone());
+    // Make deep copy of model
+    GModel* model_copy = model.clone();
+
+    // If copy is valid, insert it to the container
+    if (model_copy != NULL) {
+        m_models.insert(m_models.begin()+index, model_copy);
+    }
     // Set parameter pointers
     set_pointers();
@@ -460,10 +479,18 @@ void GModels::extend(const GModels&amp; models)
         // Reserve enough space
         reserve(size() + models.size());
-        // Loop over all model components and append deep copies to container
+        // Loop over all model components
         for (int i = 0; i < models.size(); ++i) {
-            m_models.push_back(models[i].clone());
-        }
+
+            // Make deep copy of model
+            GModel* model_copy = models[i]->clone();
+
+            // If copy is valid, append it to the container
+            if (model_copy != NULL) {
+                m_models.push_back(model_copy);
+            }
+
+        } // endfor: looped over all model components
         // Set parameter pointers
         set_pointers();
 /***************************************************************************
- *            GObservation.cpp - Abstract observation base class           *
+ *           GObservation.cpp  -  Abstract observation base class          *
  * ----------------------------------------------------------------------- *
  *  copyright (C) 2008-2013 by Juergen Knoedlseder                         *
  * ----------------------------------------------------------------------- *
@@ -190,7 +190,7 @@ double GObservation::model(const GModels&amp; models, const GEvent&amp; event,
     #endif
     // Initialise
-    double value = 0.0;    // Reset model value
+    double model = 0.0;    // Reset model value
     int    igrad = 0;      // Reset gradient counter
     // If gradient is available then reset gradient vector elements to 0
@@ -201,32 +201,35 @@ double GObservation::model(const GModels&amp; models, const GEvent&amp; event,
     // Loop over models
     for (int i = 0; i < models.size(); ++i) {
-        // Get model
-        const GModel& model = models[i];
+        // Get model pointer. Continue only if pointer is valid
+        const GModel* mptr = models[i];
+        if (mptr != NULL) {
-        // Continue only if model applies to specific instrument and
-        // observation identifier
-        if (model.isvalid(instrument(), id())) {
+            // Continue only if model applies to specific instrument and
+            // observation identifier
+            if (mptr->isvalid(instrument(), id())) {
-            // Compute value and add to model
-            value += model.eval_gradients(event, *this);
+                // Compute value and add to model
+                model += mptr->eval_gradients(event, *this);
-            // Optionally determine model gradients
-            if (gradient != NULL) {
-                for (int k = 0; k < model.size(); ++k) {
-                    (*gradient)[igrad+k] = model_grad(model, event, k);
+                // Optionally determine model gradients
+                if (gradient != NULL) {
+                    for (int k = 0; k < mptr->size(); ++k) {
+                        (*gradient)[igrad+k] = model_grad(*mptr, event, k);
+                    }
                 }
-            }
-        // Increment parameter counter for gradients
-        igrad += model.size();
+            } // endif: model component was valid for instrument
+
+            // Increment parameter counter for gradients
+            igrad += mptr->size();
         } // endif: model was valid
     } // endfor: Looped over models
-    // Return value
-    return value;
+    // Return
+    return model;
 }
@@ -273,27 +276,30 @@ double GObservation::npred(const GModels&amp; models, GVector* gradient) const
     // Loop over models
     for (int i = 0; i < models.size(); ++i) {
-        // Get model
-        const GModel& model = models[i];
+        // Get model pointer. Continue only if pointer is valid
+        const GModel* mptr = models[i];
+        if (mptr != NULL) {
-        // Continue only if model applies to specific instrument and
-        // observation identifier
-        if (model.isvalid(instrument(), id())) {
+            // Continue only if model applies to specific instrument and
+            // observation identifier
+            if (mptr->isvalid(instrument(), id())) {
-            // Determine Npred for model
-            npred += npred_temp(model);
+                // Determine Npred for model
+                npred += npred_temp(*mptr);
-            // Optionally determine Npred gradients
-            if (gradient != NULL) {
-                for (int k = 0; k < model.size(); ++k) {
-                    (*gradient)[igrad+k] = npred_grad(model, k);
+                // Optionally determine Npred gradients
+                if (gradient != NULL) {
+                    for (int k = 0; k < mptr->size(); ++k) {
+                        (*gradient)[igrad+k] = npred_grad(*mptr, k);
+                    }
                 }
-            }
-        } // endif: model component was valid for instrument
+            } // endif: model component was valid for instrument
-        // Increment parameter counter for gradient
-        igrad += model.size();
+            // Increment parameter counter for gradient
+            igrad += mptr->size();
+
+        } // endif: model was valid
     } // endfor: Looped over models
 /***************************************************************************
- *                   test_GModel.cpp - test GModel class                   *
+ *                  test_GModel.cpp  -  test GModel class                  *
  * ----------------------------------------------------------------------- *
  *  copyright (C) 2009-2013 by Juergen Knoedlseder                         *
  * ----------------------------------------------------------------------- *
@@ -193,9 +193,9 @@ void TestGModel::test_model(void)
     models2.append(crab);
     models2.save("test_instrument.xml");
     models2.load("test_instrument.xml");
-    test_value(models2[0].scale("LAT").real_value(), 1.0);
-    test_value(models2[0].scale("CTA").real_value(), 0.5);
-    test_value(models2[0].scale("COM").real_value(), 1.0);
+    test_value(models2[0]->scale("LAT").real_value(), 1.0);
+    test_value(models2[0]->scale("CTA").real_value(), 0.5);
+    test_value(models2[0]->scale("COM").real_value(), 1.0);
     // Exit test
     return;
@@ -621,7 +621,7 @@ void TestOpenMP::test_observations_optimizer(const int&amp; mode)
     obs.optimize(opt);
     // Get the result
-    GModelPar result = (obs.models()[0])[0];
+    GModelPar result = (*(obs.models()[0]))[0];
     // Check if converged
     test_assert(opt.status()==0, "Check if converged", 
@@ -122,7 +122,7 @@ void TestGOptimizer::test_optimizer(const int&amp; mode)
     obs.optimize(opt);
     // Get the result
-    GModelPar result = (obs.models()[0])[0];
+    GModelPar result = (*(obs.models()[0]))[0];
     // Check if converged
     test_assert(opt.status()==0, "Check if converged",