- IMP-dev - salilab.org

Re: [IMP-dev] various evils
by Ben Webb 18 Dec '07

18 Dec '07

Daniel Russel wrote: > I think it is a bit evil to have NDEBUG defined when building the swig > library but not any other time. Any function declared in a header can > then asserts turned off. > > Even worse, various things can change memory layout when NDEBUG is > defined, resulting in random crashes if modules are mixed. We probably > should make sure IMP doesn't have this property, but I have encountered > other libraries which do such things. I think 'evil' is a strong word for it. ;) But as of r654, there is a 'release' scons option. By default it is False, which will build both regular libraries and SWIG extensions without -DNDEBUG. If you turn release on, it'll set NDEBUG for everything. But we definitely shouldn't have different memory layouts in the two cases - that is a really sloppy ABI. > On a separate note, since it has been bugging me a bit, you are not > supposed to use identifiers starting with _ or __ as these are reserved > for the compiler. All our header guards start with __. Frankly, I don't > expect this to ever cause problems, but such are the rules. A huge number of libraries use __ for their header guards, so I'd be very surprised if this were to ever cause problems. For example, freetype, gmp, mpfr and glib on my Mac use __. A bunch of others use a single _, and those with _ or __ are a distinct minority (the situation is even more pronounced for the stuff in /usr/include). So I think you can sleep comfortably. Ben -- ben(a)salilab.org http://salilab.org/~ben/ "It is a capital mistake to theorize before one has data." - Sir Arthur Conan Doyle

1 0

Suggested IMP cleanups
by Daniel Russel 18 Dec '07

18 Dec '07

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.

2 2

Annoyance with current restraint design
by Daniel Russel 18 Dec '07

18 Dec '07

I have some restraints which I want to have be semi-dynamic, meaning, I want to be able to reset the list of particles which they act on. This doesn't work so cleanly with restraints taking lists of ParticleIndexes as then you need one version of set_particles to be called from the constructor (where the Restraint::model_ is not set) and another version to be called by outsiders after the model pointer has been set (or pass the model pointer to both and ignore the stored pointer). This seems like a reason for writing restraints which take Particles rather than ParticleIndexes. It may also be worth dropping the Restraint::model_ pointer entirely and just using the pointer stored in particles. So DistanceRestraint::DistanceRestraint(Particle*, Particle*, ScoreFunc*) ExclusionVolumeRestraint(Particles, ScoreFuncParams) Any thoughts?

2 2

Removing superfluous Model*
by Daniel Russel 18 Dec '07

18 Dec '07

As mentioned yesterday I removed the superfluous Model * from DistanceRestraint and SphericalRestraint. API changes: DistanceRestaint::DistanceRestraint and SphericalRestraint::SphericalRestraint no longer take a Model* as the first argument.

1 0

Common object base
by Daniel Russel 18 Dec '07

18 Dec '07

I was worried that I was having issues with python prematurely deleting objects so I added a common base class on the heap allocated objects (to replace boost::noncopyable). This class enforces the non- copyability as well as uses a conservative heuristic to keep track of when the object has been deleted but is still in use and to try to throw an exception when such has been detected. We don't currently have issues with object ownership in IMP, but they are a pain to track down when they occur as python generally just unceremoniously bombs. And it gets rid of the funny IMP/boost files. A few other bounds checks and null pointer checks have been added also. There are no API changes associated with this patch.

2 1

change IMP.ParticleVector to IMP.Partcles
by Daniel Russel 17 Dec '07

17 Dec '07

to be consistent with everything else (I don't like having the name Vector in python code as it acts like a python list). And the name Particles is already used for it in C++. Changed the distance test case to use it.

1 0

Add square function
by Daniel Russel 16 Dec '07

16 Dec '07

I added a template function square to utility.h which computes the square of any object. In order to make sure it gets tested I cleaned up the implementation of DistanceRestraint and made it use the square function. Since python has the ** operator already, I don't see any reason to export this function to python at the moment.

1 0

Re: [IMP-dev] Update SphericalRestraint
by Ben Webb 16 Dec '07

16 Dec '07

Daniel Russel wrote: > I think we may want to think about removing the list of particles from > the Restraint base class and providing several restraint subclasses > which use different containers to store the particles (this would be a > fairly minor change given the IMP_RESTRAINT macro and the current > interface). Then, we could have restraints which require a dynamic set > use a std::set, and ones which just have a static list or (like distance > restraint) a fixed number, use a vector or just member variables. Agreed - although I suggest there's little point in doing that unless we have specific restraints in mind which need such functionality. > I have ended up rolling my own version of ExclusionVolume. I have a > version which either enforces excluded volume, or, if the particles are > bonded, enforces the bond (it uses the BondDecorator def of bond). I > suspect something on those lines will be more useful in the long run as > we rarely have a set of particles where nothing is interacting tightly. As you mention elsewhere, this is probably pretty slow. The usual way of doing such a thing efficiently (at least in molecular mechanics packages) is to have two restraints - one is the stereochemical restraint, a simple loop over all bonds (in the Modeller case, this is done just once and restraints are set up for each bond) while the second is the excluded volume restraint, a loop over all nonbonded pairs. Part of the nonbonded pair generation mechanism excludes bonded particles. Ben -- ben(a)salilab.org http://salilab.org/~ben/ "It is a capital mistake to theorize before one has data." - Sir Arthur Conan Doyle

2 3

[Fwd: Build system failure, 12/16/2007]
by Ben Webb 16 Dec '07

16 Dec '07

Looks like Daniel's static variable initialization check caught some badness on IMP Windows builds (see below; the x86_64 build also failed, but I suspect that's an intermittent NFS issue). Patches to fix this are welcomed. ;) P.S. I suspect most of you don't have access to a Windows box with MSVC and scons installed. So to build IMP with MSVC, build it on synth and pass 'wine=true' to your scons invocation. That'll run the MS tools on Linux using Wine. Ben -------- Original Message -------- Subject: Build system failure, 12/16/2007 Date: Sun, 16 Dec 2007 07:00:02 -0800 From: build-system(a)salilab.org To: ben(a)salilab.org Build system failed on 12/16/2007. Please see https://salilab.org/internal/imp/tests.html for full details. -- ben(a)salilab.org http://salilab.org/~ben/ "It is a capital mistake to theorize before one has data." - Sir Arthur Conan Doyle

1 0

First significant code review
by Daniel Russel 14 Dec '07

14 Dec '07

I changed the interfaces of DistanceRestraint and ExclusionRestraint to make them more consistent and more general. Warning, it is quite long due to many similar changes in the test cases. Yeah for sed, but I wouldn't want to read through the diff :-) DistanceRestraint now takes a pair of ParticleIndexes and a ScoreFunc which it then owns. I also removed the constructor with a passed radius as that does add any power and adds another path which has to be tested and maintained. ExclusionRestraint takes a list or lists of ParticleIndexes (instead of ints). Various unused member variables are removed. Restraint::show now has a default argument so it can be called from python. Applying it requires first applying the decorators patch I sent before (since I removed the redundant creation of the FloatKeys in DistanceRestraint). It might also require the more minor kernel patch also included.

2 6