V3 parameter of torsions not following the gradient for absolute values less than 0.1

[jeherr]

Per @peonor and Emily (does she have a github account to link here?)

For the included FF, Q2MM completes one loop and does not find any better sets of parameters. However, when I take the v3 term on line 1966, which starts at -0.0232, and make it more negative, the score improves. None of the trial FFs have this parameter getting more negative. If I then set the parameter to -0.1 and give it an upper limit of -0.1, instead of only stepping in the negative direction, Q2MM changes the step size from 0.1 to 0.0, so the parameter stays set at its upper limit and once again, no better sets of parameters are found.

[jeherr]

I'm not sure yet, but I believe this is likely related to #51 which was closed by #53. As I remember we discussed letting the torsion parameters be free to take values between -0.1 and 0.1, and only to throw a warning at the end of the FF optimization if the torsion parameters remained within that range, but in the end I think we realized that would cause an issue with cases like setting a torsion parameter to a really large value to keep the coordination around a metal center to be square-planar. In those cases a hard limit had to be set as I recall, otherwise the optimization would waste a lot of time on those large torsion parameters. It might be time to reconsider making hard parameter limits like in those cases, and soft parameter limits where we don't want a parameter to end in a certain range, but we don't throw an exception or alter the gradient step size if it falls in that range during the optimization, just a warning at the end.

Otherwise, we'll really have to take a look at this and think about all the possibilities and how they should be handled. I think there are four basic possibilities for torsions: the parameter is at 0.1 and the gradient points in the positive direction, or the negative direction, or the parameter is at -0.1 and the gradient points in the positive direction or the negative direction.

[jeherr]

Ok so this is related to #51, which as we discussed at the end of that issue should have been fixed by letting whichever parameter step was outside the bounds be 0 and the central differentiation just simplifies to one-directional differentiation. The problem was I didn't consider the logic of changing the step size well enough. Here is the code where step sizes get changed if they fall outside the range. The issue is the step size depends on BOTH the forward and backward differentiated parameter being within the range, so that they both must take the same step size. So if one parameter is at a hard limit, the only step size allowed is 0 for both directions. To fix this, the forward and backwards differentiated parameters have to be allowed to take different step sizes. The try/except block needs to be split into two, one for the forward, and one for the backward step independently. Additionally it might be easier to just say, try taking the step, and if there is an exception because it falls out of the allowed range, just set that step to 0 for only that forward or backward param. This would only occur when the param is within 0.1 of a limit and it would just collapse to forward/backward differentiation in only those cases.

[ericchansen]

If you make the forward and backwards step size different, you're not doing central differentiation. That means we'd have to write a new optimization algorithm (or at least touch up the existing one).

I forget, but shouldn't we default to forwards or backwards differentiation in this case? Or is that up the user to do when required?

[jeherr]

@ericchansen Check out @peonor's last comment from #51. If you look at the equation for central differentiation, and set one of the step sizes to 0 and keep the other step size at h, then the equation simplifies to either forward or backward differentiation within a factor of 2.

D(f(x)) = (f(x+h) - f(x-h))/2h
D(f(x)) = (f(x+h) - f(x))/2h <--- This is just the forward differentiation equation with an extra factor of 2 in the denominator.

Now we could implement it such that it handles everything properly and gets the correct denominator, but we're talking about a factor of 2 on gradients that are already numerical approximations and this will only occur when the parameter is within 0.1 of the limit. The other solution will take probably a good bit of extra code which will be significantly more complicated then this and could likely introduce more bugs which may be harder to diagnose and fix. All-in-all I think the factor of 2 won't make much difference. All it does is make the magnitude of the gradient smaller, so it's not like it's going to over-step a minimum doing this and cause problems with optimization.

[ericchansen]

That's a good point! What worries me is that https://github.com/Q2MM/q2mm/blob/master/README.md hasn't been updated in ages. I like your solution, I just hope we document it very well. I can see how a future generation of students might think, "Uh oh! That's not actually doing central differentiation! It's a bug!"

Anyway, I support either direction ... GET IT!? It's a central differentiation joke! 😉

[jeherr]

Updating the README needs done anyway, but that should be easy enough. If we want to get a little more official, documentation should be created using something like sphinx where this could be documented, but that requires everyone to write docstrings when they make new classes/functions and to do so with one style. There are several standard docstrings styles which sphinx supports for autogenerating docs.

Also, this is exactly the kind of spot in the code where comments should be used. Generally the best practice for comments in code is to write good code which is easy to read and makes obvious sense, but when you need to do something that may not be obvious upon first glance like this, then you add a comment.

[ericchansen]

I very strongly agree that Sphinx should be used. Anyway, we're getting off topic. I created issue #69 to track this separate enhancement.