From ce089ff8967ded9862513402ef9df5476e477f31 Mon Sep 17 00:00:00 2001 From: Jürgen Knödlseder Date: Thu, 21 Feb 2013 07:17:41 +0100 Subject: [PATCH] Revert "GModels access operator now returns reference instead of pointer." --- NEWS | 3 --- include/GModels.hpp | 8 ++++---- inst/com/test/test_COM.cpp | 6 +++--- inst/cta/test/test_CTA.cpp | 12 ++++++------ inst/lat/test/test_LAT.cpp | 6 +++--- inst/mwl/test/test_MWL.cpp | 6 +++--- pyext/GModels.i | 6 +++--- src/model/GModels.cpp | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------- src/obs/GObservation.cpp | 72 +++++++++++++++++++++++++++++++++++++++--------------------------------- test/test_GModel.cpp | 8 ++++---- test/test_GObservation.cpp | 2 +- test/test_GOptimizer.cpp | 2 +- 12 files changed, 119 insertions(+), 89 deletions(-) diff --git a/NEWS b/NEWS index 7c7f792..0ca6699 100644 --- a/NEWS +++ b/NEWS @@ -36,9 +36,6 @@ GOptimizer & 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 ----------------- diff --git a/include/GModels.hpp b/include/GModels.hpp index 3b71cbd..3a0ea5d 100644 --- a/include/GModels.hpp +++ b/include/GModels.hpp @@ -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); diff --git a/inst/com/test/test_COM.cpp b/inst/com/test/test_COM.cpp index eba8ba9..1260d17 100644 --- a/inst/com/test/test_COM.cpp +++ b/inst/com/test/test_COM.cpp @@ -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); diff --git a/inst/cta/test/test_CTA.cpp b/inst/cta/test/test_CTA.cpp index 4a71846..39a0c21 100644 --- a/inst/cta/test/test_CTA.cpp +++ b/inst/cta/test/test_CTA.cpp @@ -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); diff --git a/inst/lat/test/test_LAT.cpp b/inst/lat/test/test_LAT.cpp index 796a411..0dbc5b8 100644 --- a/inst/lat/test/test_LAT.cpp +++ b/inst/lat/test/test_LAT.cpp @@ -663,9 +663,9 @@ void TestGLATOptimize::test_one_binned_optimizer(const std::string& 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); diff --git a/inst/mwl/test/test_MWL.cpp b/inst/mwl/test/test_MWL.cpp index 35aa8d9..a2d96de 100644 --- a/inst/mwl/test/test_MWL.cpp +++ b/inst/mwl/test/test_MWL.cpp @@ -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); diff --git a/pyext/GModels.i b/pyext/GModels.i index 3e69368..1dff223 100644 --- a/pyext/GModels.i +++ b/pyext/GModels.i @@ -32,7 +32,7 @@ %} /* __ Typemaps ___________________________________________________________ */ -%typemap(out) GModel& { +%typemap(out) GModel* { if (dynamic_cast($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) { diff --git a/src/model/GModels.cpp b/src/model/GModels.cpp index 9994d79..a371b8d 100644 --- a/src/model/GModels.cpp +++ b/src/model/GModels.cpp @@ -162,16 +162,17 @@ GModels& GModels::operator= (const GModels& 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& GModels::operator[](const int& 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& GModels::operator[](const int& 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& GModels::operator[](const std::string& 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& GModels::operator[](const std::string& 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& index, const GModel& 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& name, const GModel& 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& 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(); diff --git a/src/obs/GObservation.cpp b/src/obs/GObservation.cpp index 3fb97be..670f7bc 100644 --- a/src/obs/GObservation.cpp +++ b/src/obs/GObservation.cpp @@ -1,5 +1,5 @@ /*************************************************************************** - * 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& models, const GEvent& 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& models, const GEvent& 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& 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 diff --git a/test/test_GModel.cpp b/test/test_GModel.cpp index 58f6986..b720d07 100644 --- a/test/test_GModel.cpp +++ b/test/test_GModel.cpp @@ -1,5 +1,5 @@ /*************************************************************************** - * 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; diff --git a/test/test_GObservation.cpp b/test/test_GObservation.cpp index 7155e84..5913677 100644 --- a/test/test_GObservation.cpp +++ b/test/test_GObservation.cpp @@ -621,7 +621,7 @@ void TestOpenMP::test_observations_optimizer(const int& 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", diff --git a/test/test_GOptimizer.cpp b/test/test_GOptimizer.cpp index 2ae65e2..aa35ff5 100644 --- a/test/test_GOptimizer.cpp +++ b/test/test_GOptimizer.cpp @@ -122,7 +122,7 @@ void TestGOptimizer::test_optimizer(const int& 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", -- libgit2 0.21.4