On 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. The function returns two equivalent return values two different ways, breaks if you happen to call your argument something different and looks completely different in python and c.
> 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/ > "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()): > tions */ > %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()): > t_is_active()): > _______________________________________________ > IMP-dev mailing list > IMP-dev@salilab.org > https://salilab.org/mailman/listinfo/imp-dev