IMP-dev

imp-dev@salilab.org

868 discussions

Start a nNew thread

Changing vectori vectorf
by Daniel Russel 14 Dec '07

14 Dec '07

I propose we change IMP.vectori and IMP.vectorf to IMP.Ints and IMP.Floats to be consistent. Any objections?

3 3

Minor changes to xyzdecorator
by Daniel Russel 14 Dec '07

14 Dec '07

Here are some minor changes to the XYZDecorator. It now always creates the keys since we are always using the. This means that the get_coordinate_key(i) method doesn't have to check when getting the keys out. It also has methods to change whether the particle is optimized or not. These methods are checked as part of the SphereRestraint patch I will submit separately.

1 0

CoordinateRestraint
by Daniel Russel 14 Dec '07

14 Dec '07

It seems entirely useless to me as written as it can only restrict coordinates relative to the origin. I propose getting rid of it and instead providing, as needed, SphericalDistanceRestraint (restraint the distance from a fixed point), CoordinateRestraint (which restricts a coordinate relative to a fixed value) etc. The names could use some work. My suggestion for SphericalDistanceRestraint (which is what I want) is: SphericalDistanceRestraint(Model*, ParticleIndex (or Particle*), Float x, Float y, Float z, ScoreFunc *f); Then you can enforce any spherically symmetric restraint on a set of particles (in my case enforcing that they are all in some sphere bounding the system).

2 4

Re: [IMP-dev] Fix behavior of DistanceRestraint at near-zero distance
by Ben Webb 13 Dec '07

13 Dec '07

Daniel Russel wrote: > Fine with me with a few comments. > > I don't know that counting on pre-optimization perturbation is > reasonable as a degeneracy could arise during an optimization. That > said, in most cases, other forces should cause the two particles to > diverge even if the degenerate distance restraint does not. Agreed. The probability of two particles ending up in exactly the same position is rather small (unless you have a force pulling them together). > In Bret's defense, his code returned random derivatives, not a random > score as he snapped the distance to the minimum. You are, of course, exactly correct. 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

Fix behavior of DistanceRestraint at near-zero distance
by Ben Webb 13 Dec '07

13 Dec '07

Attached is a patch which corrects the math in DistanceRestraint. Currently if the interparticle distance is zero (or very near zero) DistanceRestraint pretends that the particles are randomly placed in near proximity, so that a random distance and derivatives are returned. This is because the derivatives of the distance at zero are undefined (the math is 0/0). IMHO, this behavior is flawed, because the score of a given state should always be the same - and the scoring function should be deterministic. Any necessary randomization of the system should be introduced by perturbing the system state before an optimization, not by having the scoring function return random values. (The current behavior also makes it impossible to introduce a distance restraint with zero mean, to force two particles to be colocated.) This patch returns a zero derivative for zero distance, which is at least consistent with every molecular mechanics package I've seen. 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

Re: [IMP-dev] First code review
by Ben Webb 13 Dec '07

13 Dec '07

Daniel Russel wrote: >> They come and complain to me in person. And it is hard to object to a >> change which isn't mentioned on the list, until much later when you >> realize your code doesn't work any more. > Recently? I haven't heard anyone complain in rather a while a month or so. That's because they complain only to me. > I agree that more should be on the list. I stopped emailing the list > because I was just getting complaints about their being too much traffic > on the list and no one else was contributing. Anyway, it should resume. If people think there's too much traffic on the list, they don't have to read the emails. But then they shouldn't complain either. ;) Most importantly, the list emails are archived and publicly accessible, while my personal email is not. So even someone that doesn't read the list can be pointed to a URL if they complain about something. >> You can have discussion before or discussion afterwards. It's your >> choice. But just going ahead and writing the code is quite annoying if >> it conflicts with code other people are writing in the meantime. > Is there code outside of new_imp/ and the EM code that depends on it? We have no way of telling, of course, because anyone is welcome to write their own private modules. I don't _know_ of anybody doing that right now, but we are just starting to add non-lab people to IMP, so I'm sure it'll become more common. 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

Re: [IMP-dev] First code review
by Ben Webb 13 Dec '07

13 Dec '07

Daniel Russel wrote: >> You can't really expect changes to be committed rapidly if you make them >> before discussing the implications, because that requires everyone to >> agree with everything you do, which is not likely to be the case. ;) > Well, I don't think we have ever had an objection from anyone else :-) They come and complain to me in person. And it is hard to object to a change which isn't mentioned on the list, until much later when you realize your code doesn't work any more. > Discussing thing beforehand doesn't help anyway, since it requires that > I stop what I am doing and work on something else while the discussion > occurs rather than just making the changes. You can have discussion before or discussion afterwards. It's your choice. But just going ahead and writing the code is quite annoying if it conflicts with code other people are writing in the meantime. > Sure. Much of the problem is the frequency of changes, not the latency > of the commit time (although the unpredictability of the commit time is > sometimes annoying). I am not always complaining about you. Just > sometimes :-) You can only predict the commit time if you can predict whether I'll agree with your changes. One way to know that is to mention them on the list before you make them. 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

Re: [IMP-dev] First code review
by Ben Webb 13 Dec '07

13 Dec '07

Daniel Russel wrote: > The problem is, I can't currently write very clean patches. I have to > produce them from overlapping changes I have already made. If it were > the case that the response time to the change were fast compared to the > frequency of these changes, then they could be cleaner. It will happen > eventually as things stabilize, just not quite yet. You can't really expect changes to be committed rapidly if you make them before discussing the implications, because that requires everyone to agree with everything you do, which is not likely to be the case. ;) If patches are clean, implement something we've agreed on doing, and are consistent with the style guidelines, they can go in more or less instantly. If they don't apply cleanly, make 'scons standards' complain, or make huge changes to the interface without any kind of prior warning, you can hardly be surprised if they get delayed a little. 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

Re: [IMP-dev] First significant code review
by Ben Webb 13 Dec '07

13 Dec '07

Daniel Russel wrote: [re: default argument for Restraint::show()] >> I didn't see a unit test for this, but I guess I can write one for you >> when I commit it. > Well, I couldn't quite figure out anything useful to test especially as > the stream gets dumped to /dev/null :-) You can test that the method can be called, which is exactly what r634 does. I don't see a lot of value in testing what it prints, since it's really only for human consumption anyway, but that can wait for a Python file object to C++ stream interface. 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

Re: [IMP-dev] First code review
by Ben Webb 13 Dec '07

13 Dec '07

Daniel Russel wrote: > On Dec 13, 2007, at 11:41 AM, Ben Webb wrote: >> Coverage is one thing, but a simple >> unit test that directly accompanies a new piece of code helps to >> document the intent of that code (e.g. in a submitted patch). >> >> But unit tests are not documentation >> (sure, it'd be nice if they were readable enough to serve as simple >> examples, but tests and examples are two rather different things). > > You above say that unit tests help to document the intent and below say > that they are not documentation. I agree with the later and think trying > to do it with the former just creates excess code with no purpose. I don't see any inconsistency there - we're documenting different things. Unit tests are not documentation in themselves, but do provide some indication of the intent of a method which is useful in code review (particularly if the accompanying description of the patch is lacking or the doxygen comments are incomplete). I'm simply talking about test-driven coding here. Documentation is great for saying "foo_method takes a positive integer and returns its square, or RangeError on error" but a unit test that invokes that method and asserts that RangeError is thrown for a negative integer actually enforces that. That's not "excess code with no purpose" because the documentation does not ensure that happens. > As a side note, having the test code in an unrelated patch is generally > a bad idea, I don't know why you say "as a side note", because that's the point I'm making here. > but separating out changes into clean patches is not always > easy and in this case would have involved significantly more work. Sure, which is why it's important to have a code review policy in mind before writing such patches. 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

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

IMP-dev