Well, the set of all patches runs passes the tests on flute when run in valgrind (other than the dna read which fails for a virgin svn copy). And all pass (without valgrind) on my mac. I suspect the problem is changes you made to the patches I submitted, but it is also possible that the patches simply expose bugs created by the problems in attribute table that a later patch fixed. Either way, you have a working set of patches.
On Tue, Aug 26, 2008 at 7:55 PM, Ben Webb ben@salilab.org wrote:
> Daniel Russel wrote: > > Here is the patch that was discussed a while ago which makes > > UnaryFunction::evaluate_with_derivative return a std::pair instead of > > using one pass by reference to return. It also removes the hack in IMP.i > > to get the return value right. > > I'm not sure sure why you call it a hack, since it's the regular way to > deal with reference return values in SWIG. And that code deals with any > errors the Python API (or any other scripting language) may return. But > regardless, the code (patch attached) does not work on synth: > > % scons test > ... > RuntimeError: Previously freed object is not valid: 0x89c1224 > File "kernel/src/Object.cpp", line 32 > > unit tests FAILED > > Ben > -- > ben@salilab.org http://salilab.org/~ben/http://salilab.org/%7Eben/ > "It is a capital mistake to theorize before one has data." > - Sir Arthur Conan Doyle > > Index: kernel/include/IMP/UnaryFunction.h > =================================================================== > --- kernel/include/IMP/UnaryFunction.h (revision 669) > +++ kernel/include/IMP/UnaryFunction.h (working copy) > @@ -14,9 +14,13 @@ > namespace IMP > { > > +typedef std::pair<Float, Float> FloatPair; > + > //! Abstract single variable functor class for score functions. > /** These functors take a single feature value, and return a corresponding > score (and optionally also the first derivative). > + > + \ingroup kernel > */ > class IMPDLLEXPORT UnaryFunction : public RefCountedObject > { > @@ -36,7 +40,7 @@ > given feaure. > \return Score > */ > - virtual Float evaluate_with_derivative(Float feature, Float& deriv) > const = 0; > + virtual FloatPair evaluate_with_derivative(Float feature) const = 0; > > virtual void show(std::ostream &out=std::cout) const = 0; > }; > Index: kernel/include/IMP/internal/evaluate_distance_pair_score.h > =================================================================== > --- kernel/include/IMP/internal/evaluate_distance_pair_score.h (revision > 669) > +++ kernel/include/IMP/internal/evaluate_distance_pair_score.h (working > copy) > @@ -9,6 +9,7 @@ > #define __IMP_EVALUATE_DISTANCE_PAIR_SCORE_H > > #include "../Vector3D.h" > +#include <boost/tuple/tuple.hpp> > > namespace IMP > { > @@ -43,7 +44,7 @@ > if (da && distance >= MIN_DISTANCE) { > Float deriv; > > - score = f->evaluate_with_derivative(shifted_distance, deriv); > + boost::tie(score, deriv) = > f->evaluate_with_derivative(shifted_distance); > > Vector3D d= delta/distance *deriv; > d0.add_to_coordinates_derivative(d, *da); > Index: kernel/include/IMP/unary_functions/Linear.h > =================================================================== > --- kernel/include/IMP/unary_functions/Linear.h (revision 669) > +++ kernel/include/IMP/unary_functions/Linear.h (working copy) > @@ -28,9 +28,8 @@ > return (feature-offset_)*slope_; > } > > - virtual Float evaluate_with_derivative(Float feature, Float& deriv) > const { > - deriv= slope_; > - return evaluate(feature); > + virtual FloatPair evaluate_with_derivative(Float feature) const { > + return std::make_pair(evaluate(feature), slope_); > } > > void set_slope(Float f) { > Index: kernel/include/IMP/unary_functions/ClosedCubicSpline.h > =================================================================== > --- kernel/include/IMP/unary_functions/ClosedCubicSpline.h (revision > 669) > +++ kernel/include/IMP/unary_functions/ClosedCubicSpline.h (working > copy) > @@ -40,12 +40,10 @@ > > //! Calculate score and derivative with respect to the given feature. > /** \param[in] feature Value of feature being tested. > - \param[out] deriv Partial derivative of the score with respect to > - the feature value. > \exception ValueException Feature is out of defined range. > \return Score > */ > - virtual Float evaluate_with_derivative(Float feature, Float& deriv) > const; > + virtual FloatPair evaluate_with_derivative(Float feature) const; > > void show(std::ostream &out=std::cout) const { > out << "Closed cubic spline of " << values_.size() << " values from " > Index: kernel/include/IMP/unary_functions/WormLikeChain.h > =================================================================== > --- kernel/include/IMP/unary_functions/WormLikeChain.h (revision 669) > +++ kernel/include/IMP/unary_functions/WormLikeChain.h (working copy) > @@ -62,10 +62,8 @@ > > //! Calculate the WormLikeChain energy given the length > /** \param[in] l Current length in angstroms > - \param[out] deriv force in kcal/angstrom mol > - \return Score > */ > - virtual Float evaluate_with_derivative(Float fl, Float& deriv) const { > + virtual FloatPair evaluate_with_derivative(Float fl) const { > unit::Angstrom l(fl); > if (l < unit::Angstrom(0)) l=unit::Angstrom(0); > unit::Piconewton doubled; > @@ -81,9 +79,9 @@ > // convert from picoNewton > unit::YoctoKilocaloriePerAngstrom du= unit::convert_J_to_Cal(doubled); > > - deriv = (du*unit::ATOMS_PER_MOL).get_value(); > + Float deriv = (du*unit::ATOMS_PER_MOL).get_value(); > //std::cout << "Which converts to " << d << std::endl; > - return evaluate(fl); > + return std::make_pair(evaluate(fl), deriv); > } > > void show(std::ostream &out=std::cout) const { > Index: kernel/include/IMP/unary_functions/Harmonic.h > =================================================================== > --- kernel/include/IMP/unary_functions/Harmonic.h (revision 669) > +++ kernel/include/IMP/unary_functions/Harmonic.h (working copy) > @@ -56,20 +56,17 @@ > \return Score > */ > virtual Float evaluate(Float feature) const { > - Float d; > - return evaluate_with_derivative(feature, d); > + return evaluate_with_derivative(feature).first; > } > > //! Calculate harmonic score and derivative with respect to the given > feature. > /** \param[in] feature Value of feature being tested. > - \param[out] deriv Partial derivative of the score with respect to > - the feature value. > \return Score > */ > - virtual Float evaluate_with_derivative(Float feature, Float& deriv) > const { > + virtual FloatPair evaluate_with_derivative(Float feature) const { > Float e = (feature - mean_); > - deriv = k_ * e; > - return 0.5 * k_ * e * e; > + Float deriv = k_ * e; > + return std::make_pair(0.5 * k_ * e * e, deriv); > } > > void show(std::ostream &out=std::cout) const { > Index: kernel/include/IMP/unary_functions/Cosine.h > =================================================================== > --- kernel/include/IMP/unary_functions/Cosine.h (revision 669) > +++ kernel/include/IMP/unary_functions/Cosine.h (working copy) > @@ -43,11 +43,9 @@ > > //! Calculate score and derivative with respect to the given feature. > /** \param[in] feature Value of feature being tested. > - \param[out] deriv Partial derivative of the score with respect to > - the feature value. > \return Score > */ > - virtual Float evaluate_with_derivative(Float feature, Float& deriv) > const; > + virtual FloatPair evaluate_with_derivative(Float feature) const; > > void show(std::ostream &out=std::cout) const { > out << "Cosine function with force " << force_constant_ > Index: kernel/include/IMP/unary_functions/HarmonicLowerBound.h > =================================================================== > --- kernel/include/IMP/unary_functions/HarmonicLowerBound.h (revision > 669) > +++ kernel/include/IMP/unary_functions/HarmonicLowerBound.h (working > copy) > @@ -37,16 +37,13 @@ > //! Calculate lower-bound harmonic score and derivative for a feature. > /** If the feature is greater than or equal to the mean, the score is > zero. > \param[in] feature Value of feature being tested. > - \param[out] deriv Partial derivative of the score with respect to > - the feature value. > \return Score > */ > - virtual Float evaluate_with_derivative(Float feature, Float& deriv) > const { > + virtual FloatPair evaluate_with_derivative(Float feature) const { > if (feature >= Harmonic::get_mean()) { > - deriv = 0.0; > - return 0.0; > + return std::make_pair(0.0f, 0.0f); > } else { > - return Harmonic::evaluate_with_derivative(feature, deriv); > + return Harmonic::evaluate_with_derivative(feature); > } > } > > Index: kernel/include/IMP/unary_functions/OpenCubicSpline.h > =================================================================== > --- kernel/include/IMP/unary_functions/OpenCubicSpline.h (revision > 669) > +++ kernel/include/IMP/unary_functions/OpenCubicSpline.h (working > copy) > @@ -25,7 +25,7 @@ > \param[in] minrange Feature value at first spline point > \param[in] spacing Distance (in feature space) between points > */ > - OpenCubicSpline(const std::vector<Float> &values, Float minrange, > + OpenCubicSpline(const Floats &values, Float minrange, > Float spacing); > > virtual ~OpenCubicSpline() {} > @@ -39,12 +39,10 @@ > > //! Calculate score and derivative with respect to the given feature. > /** \param[in] feature Value of feature being tested. > - \param[out] deriv Partial derivative of the score with respect to > - the feature value. > \exception ValueException Feature is out of defined range. > \return Score > */ > - virtual Float evaluate_with_derivative(Float feature, Float& deriv) > const; > + virtual FloatPair evaluate_with_derivative(Float feature) const; > > void show(std::ostream &out=std::cout) const { > out << "Open cubic spline of " << values_.size() << " values from " > Index: kernel/include/IMP/unary_functions/HarmonicUpperBound.h > =================================================================== > --- kernel/include/IMP/unary_functions/HarmonicUpperBound.h (revision > 669) > +++ kernel/include/IMP/unary_functions/HarmonicUpperBound.h (working > copy) > @@ -37,16 +37,13 @@ > //! Calculate upper-bound harmonic score and derivative for a feature. > /** If the feature is less than or equal to the mean, the score is zero. > \param[in] feature Value of feature being tested. > - \param[out] deriv Partial derivative of the score with respect to > - the feature value. > \return Score > */ > - virtual Float evaluate_with_derivative(Float feature, Float& deriv) > const { > + virtual FloatPair evaluate_with_derivative(Float feature) const { > if (feature <= Harmonic::get_mean()) { > - deriv = 0.0; > - return 0.0; > + return std::make_pair(0.0f, 0.0f); > } else { > - return Harmonic::evaluate_with_derivative(feature, deriv); > + return Harmonic::evaluate_with_derivative(feature); > } > } > > Index: kernel/src/singleton_scores/AttributeSingletonScore.cpp > =================================================================== > --- kernel/src/singleton_scores/AttributeSingletonScore.cpp (revision > 669) > +++ kernel/src/singleton_scores/AttributeSingletonScore.cpp (working > copy) > @@ -8,7 +8,7 @@ > #include "IMP/singleton_scores/AttributeSingletonScore.h" > #include "IMP/UnaryFunction.h" > #include "IMP/Particle.h" > - > +#include <boost/tuple/tuple.hpp> > namespace IMP > { > > @@ -20,8 +20,8 @@ > DerivativeAccumulator *da) const > { > if (da) { > - Float d; > - float r= f_->evaluate_with_derivative(b->get_value(k_), d); > + Float d, r; > + boost::tie(d,r) = f_->evaluate_with_derivative(b->get_value(k_)); > b->add_to_derivative(k_, d, *da); > return r; > } else { > Index: kernel/src/singleton_scores/TunnelSingletonScore.cpp > =================================================================== > --- kernel/src/singleton_scores/TunnelSingletonScore.cpp (revision > 669) > +++ kernel/src/singleton_scores/TunnelSingletonScore.cpp (working > copy) > @@ -8,7 +8,7 @@ > > #include "IMP/singleton_scores/TunnelSingletonScore.h" > #include "IMP/decorators/XYZDecorator.h" > - > +#include <boost/tuple/tuple.hpp> > namespace IMP > { > > @@ -58,7 +58,7 @@ > // look below if changed > Float dist= -std::min(std::min(rd, hdu), hdd) - radius; > if (accum) { > - score= f_->evaluate_with_derivative(dist, deriv_scalar); > + boost::tie(score, deriv_scalar)= > f_->evaluate_with_derivative(dist); > } else { > score= f_->evaluate(dist); > } > Index: kernel/src/unary_functions/Cosine.cpp > =================================================================== > --- kernel/src/unary_functions/Cosine.cpp (revision 669) > +++ kernel/src/unary_functions/Cosine.cpp (working copy) > @@ -18,11 +18,11 @@ > - force_constant_ * std::cos(periodicity_ * feature + phase_); > } > > -Float Cosine::evaluate_with_derivative(Float feature, Float& deriv) const > +FloatPair Cosine::evaluate_with_derivative(Float feature) const > { > - deriv = force_constant_ * periodicity_ > - * std::sin(periodicity_ * feature + phase_); > - return evaluate(feature); > + Float deriv = force_constant_ * periodicity_ > + * std::sin(periodicity_ * feature + phase_); > + return std::make_pair(evaluate(feature), deriv); > } > > } // namespace IMP > Index: kernel/src/unary_functions/OpenCubicSpline.cpp > =================================================================== > --- kernel/src/unary_functions/OpenCubicSpline.cpp (revision 669) > +++ kernel/src/unary_functions/OpenCubicSpline.cpp (working copy) > @@ -66,8 +66,7 @@ > * (spacing_ * spacing_) / 6.; > } > > -Float OpenCubicSpline::evaluate_with_derivative(Float feature, > - Float& deriv) const > +FloatPair OpenCubicSpline::evaluate_with_derivative(Float feature) const > { > size_t lowbin = static_cast<size_t>((feature - minrange_) / spacing_); > // handle the case where feature ~= maxrange > @@ -79,11 +78,11 @@ > Float a = 1. - b; > float sixthspacing = spacing_ / 6.; > > - deriv = (values_[highbin] - values_[lowbin]) / spacing_ > - - (3. * a * a - 1.) * sixthspacing * second_derivs_[lowbin] > - + (3. * b * b - 1.) * sixthspacing * second_derivs_[highbin]; > + Float deriv = (values_[highbin] - values_[lowbin]) / spacing_ > + - (3. * a * a - 1.) * sixthspacing * second_derivs_[lowbin] > + + (3. * b * b - 1.) * sixthspacing * second_derivs_[highbin]; > > - return evaluate(feature); > + return std::make_pair(evaluate(feature), deriv); > } > > } // namespace IMP > Index: kernel/src/unary_functions/ClosedCubicSpline.cpp > =================================================================== > --- kernel/src/unary_functions/ClosedCubicSpline.cpp (revision 669) > +++ kernel/src/unary_functions/ClosedCubicSpline.cpp (working copy) > @@ -82,8 +82,8 @@ > * (spacing_ * spacing_) / 6.; > } > > -Float ClosedCubicSpline::evaluate_with_derivative(Float feature, > - Float& deriv) const > +FloatPair > +ClosedCubicSpline::evaluate_with_derivative(Float feature) const > { > size_t lowbin = static_cast<size_t>((feature - minrange_) / spacing_); > size_t highbin = lowbin + 1; > @@ -99,11 +99,11 @@ > Float a = 1. - b; > float sixthspacing = spacing_ / 6.; > > - deriv = (values_[highbin] - values_[lowbin]) / spacing_ > + Float deriv = (values_[highbin] - values_[lowbin]) / spacing_ > - (3. * a * a - 1.) * sixthspacing * second_derivs_[lowbin] > + (3. * b * b - 1.) * sixthspacing * second_derivs_[highbin]; > > - return evaluate(feature); > + return std::make_pair(evaluate(feature), deriv); > } > > } // namespace IMP > Index: kernel/src/restraints/DihedralRestraint.cpp > =================================================================== > --- kernel/src/restraints/DihedralRestraint.cpp (revision 669) > +++ kernel/src/restraints/DihedralRestraint.cpp (working copy) > @@ -15,6 +15,8 @@ > #include "IMP/restraints/DihedralRestraint.h" > #include "IMP/decorators/XYZDecorator.h" > > +#include <boost/tuple/tuple.hpp> > + > namespace IMP > { > > @@ -80,7 +82,7 @@ > > if (accum) { > Float deriv; > - score = score_func_->evaluate_with_derivative(angle, deriv); > + boost::tie(score, deriv) = > score_func_->evaluate_with_derivative(angle); > > // method for derivative calculation from van Schaik et al. > // J. Mol. Biol. 234, 751-762 (1993) > Index: kernel/src/triplet_scores/AngleTripletScore.cpp > =================================================================== > --- kernel/src/triplet_scores/AngleTripletScore.cpp (revision 669) > +++ kernel/src/triplet_scores/AngleTripletScore.cpp (working copy) > @@ -8,7 +8,7 @@ > #include "IMP/triplet_scores/AngleTripletScore.h" > #include "IMP/decorators/XYZDecorator.h" > #include "IMP/UnaryFunction.h" > - > +#include <boost/tuple/tuple.hpp> > namespace IMP > { > > @@ -46,7 +46,7 @@ > > if (da) { > Float deriv; > - score = f_->evaluate_with_derivative(angle, deriv); > + boost::tie(score, deriv) = f_->evaluate_with_derivative(angle); > > Vector3D unit_rij = rij.get_unit_vector(); > Vector3D unit_rkj = rkj.get_unit_vector(); > Index: kernel/pyext/IMP.i > =================================================================== > --- kernel/pyext/IMP.i (revision 672) > +++ kernel/pyext/IMP.i (working copy) > @@ -14,8 +14,17 @@ > > /* Return derivatives from unary functions */ > %include "typemaps.i" > -%apply double &OUTPUT { IMP::Float& deriv }; > > +namespace IMP { > + %typemap(out) std::pair<Float,Float> { > + PyObject *tup= PyTuple_New(2); > + PyTuple_SetItem(tup, 0, PyFloat_FromDouble($1.first)); > + PyTuple_SetItem(tup, 1, PyFloat_FromDouble($1.second)); > + $result= tup; > + } > +} > + > + > %pythoncode %{ > def check_particle(p, a): > if (not p.get_is_active()): > > _______________________________________________ > IMP-dev mailing list > IMP-dev@salilab.org > https://salilab.org/mailman/listinfo/imp-dev > >