I would like to propose getting rid of the trajectory log in the model as we can do the same sort of thing using State classes. Since these aren't part of the Model, they would be much more flexible and customizable. I currently have such a state for writing chimera BILD files that I can submit at some point.
And I would propose removing the ModelData OptimizedIndexIterator (or whatever it is called). The API is funny and it doesn't provide any functionality which can't be achieved other ways using more or less the same number of lines of code.
Daniel Russel wrote: > I would like to propose getting rid of the trajectory log in the model > as we can do the same sort of thing using State classes. Since these > aren't part of the Model, they would be much more flexible and > customizable. I currently have such a state for writing chimera BILD > files that I can submit at some point.
Agreed. The trajectory log is very inflexible, and does not use any kind of standard format. I had also mentally targeted it for destruction. This will, of course, horribly break the Restrainer web interface, but presumably Jeremy will speak up if this is a problem for him.
> And I would propose removing the ModelData OptimizedIndexIterator (or > whatever it is called). The API is funny and it doesn't provide any > functionality which can't be achieved other ways using more or less > the same number of lines of code.
No complaints from me.
Ben
Since my untested changes broke my local copy I had to fix them. Here is a clean patch for the checks I mentioned.
Ben Webb wrote: > Daniel Russel wrote: > >> I would like to propose getting rid of the trajectory log in the model >> as we can do the same sort of thing using State classes. Since these >> aren't part of the Model, they would be much more flexible and >> customizable. I currently have such a state for writing chimera BILD >> files that I can submit at some point. >> > > Agreed. The trajectory log is very inflexible, and does not use any kind > of standard format. I had also mentally targeted it for destruction. > This will, of course, horribly break the Restrainer web interface, but > presumably Jeremy will speak up if this is a problem for him. > > >> And I would propose removing the ModelData OptimizedIndexIterator (or >> whatever it is called). The API is funny and it doesn't provide any >> functionality which can't be achieved other ways using more or less >> the same number of lines of code. >> > > No complaints from me. > > Ben >
Index: include/IMP/Particle.h =================================================================== --- include/IMP/Particle.h (revision 653) +++ include/IMP/Particle.h (working copy) @@ -13,7 +13,6 @@ #include "IMP_config.h" #include "base_types.h" #include "Model.h" -#include "Restraint.h" #include "Object.h" #include "utility.h" #include "Key.h" Index: include/IMP/Restraint.h =================================================================== --- include/IMP/Restraint.h (revision 653) +++ include/IMP/Restraint.h (working copy) @@ -13,7 +13,6 @@ #include <limits>
#include "IMP_config.h" -#include "Model.h" #include "ScoreFunc.h" #include "Particle.h" #include "DerivativeAccumulator.h" @@ -79,9 +78,7 @@ //! The model the restraint is part of. /** \param[in] model The model. */ - void set_model(Model* model) { - model_=model; - } + void set_model(Model* model);
//! Return the model containing this restraint Model *get_model() const { @@ -96,6 +93,15 @@ int add_particle(Particle *p) { IMP_assert(p != NULL, "Can't add NULL particle"); particles_.push_back(p); + IMP_assert(particles_[0]->get_model() == particles_.back()->get_model(), + "All particles in restraint must be from the same model."); + IMP_assert(particles_.back()->get_model() + ->get_particle(particles_.back()->get_index()) + == particles_.back(), + "Model does not have pointer to particle."); + IMP_assert(model_== NULL || model_==particles_.back()->get_model(), + "Restraint model pointer and particle model pointer " + << "don't match."); return particles_.size()-1; }
Index: src/Restraint.cpp =================================================================== --- src/Restraint.cpp (revision 653) +++ src/Restraint.cpp (working copy) @@ -15,7 +15,6 @@ namespace IMP {
-//! Constructor Restraint::Restraint(std::string name): name_(name) { model_ = NULL; @@ -30,27 +29,25 @@ }
-//! Set whether the restraint is active i.e. if it should be evaluated. -/** \param[in] is_active If true, the restraint is active. - */ void Restraint::set_is_active(const bool is_active) { is_active_ = is_active; }
-//! Get whether the restraint is active. i.e. if it should be evaluated. -/** \return true if the restraint is active. - */ bool Restraint::get_is_active() const { return is_active_ && are_particles_active_; }
-//! Show the current restraint. -/** \param[in] out Stream to send restraint description to. - */ +void Restraint::set_model(Model* model) { + IMP_assert(model== NULL || particles_.empty() + || model == particles_[0]->get_model(), + "Model* different from Particle Model*"); + model_=model; +} + void Restraint::show(std::ostream& out) const { if (get_is_active()) { @@ -64,10 +61,6 @@ }
-//! Check if all necessary particles are still active. -/** If not, inactivate self. Called when at least one model particle - has been inactivated. - */ void Restraint::check_particles_active() { IMP_assert(get_model() != NULL,
-
Ben Webb -
Daniel Russel