-
Notifications
You must be signed in to change notification settings - Fork 59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement fit gradients for non-scalar components (see also #61) #87
Comments
Another benefit of that would be debugGradients on non-scalar cvcs. |
This is a likely cause behind a long-standing bug reported by Chris Chipot, FE calculations on a scripted coordinate based on a vector cvc. |
Before moving on with all other things, can you reproduce the bug conclusively? |
To be precise: can you conclusively attribute the poor convergence of ABF in Chris's example to the lack of fit gradients? If that's the case, a test with a much larger |
I can try, but I only expect a larger fittingGroup to distribute the counter-force on more atoms. Wouldn't the net counter-force would stay the same, as long as the fittingGroup describes the same frame of reference? What I did though, is disable the fit gradients for a similar test where they were available, and I got similarly erroneous results. What I get from that test is that in that kind of applications (essentially receptor-ligand coordinates), the fit gradients are not optional. |
I'm clarifying the scope of the issue, until it is resolved. The error applies to PMF calculations on vector variables with an external fitting group, which do not have a published implementation yet. All current PMF calculations (which are restricted to one or more scalar variables) are unaffected. |
It also affects pmf calculations on scalar variables that are scripted functions of vector components. That is a combination of documented features. |
If the script uses vector variables it is, of course, affected. Making a vector variable into a scalar through a script doesn't change the flow of the underlying code. |
To avoid undetected incorrect behavior, the fit gradients should be available only when atomic gradients are calculated in the first place. I see two levels at which it can be done:
To me the first option seems simpler, because I don't see cases where you'd want to control the fit gradients separately for different groups in one CVC, but maybe I'm missing something. |
I also think that the first option is preferable in principle. The function |
While working on this, it becomes clear to me that it the moving frame of reference system, in all applications I'm aware of, is CVC-wide. Whenever there are several atom groups, they are all fitted to the same group. I cannot imagine a meaningful case where you'd want to use different fitting groups. In practice, it is always the CVC that is calculated in a moving frame of reference, rather than individual atomic coordinates. In that case, it is tempting to move the fitting options to the CVC altogether. As a bonus, a single fitting group would be shared by all atoms groups and the optimal rotation calculated only once. The fit gradients themselves would be summed over the parent groups, instead of accumulated separately for each group. In the case of internal coordinates, those summed fit gradients would cancel out. [The applications as far as I know:
|
A meaningful case would be a symmetry-like variable that measures differences in the internal structures of a pair of equivalent macromolecules, each tumbling independently. Weird as it may be, I'd rather not invest effort on this, since it takes a bit of work and would not allow sharing the fitting group across variables or even CVC objects, anyway. |
Fair enough! |
Commit 31a232a, solves the case where fit gradients were silently missing, unbeknownst to the user. Therefore, downgrading this issue from bug to enhancement. |
I am now thinking of this issue. Do we really need explicit gradients for the fitting group? There should be a way for applying a force on a non-scalar CVC, and then passing the force to the fitting group using the chain rule, just like |
I agree that we don't need explicit gradients as long as forces can be propagated. |
These would be easily calculated if the atomic gradients were stored in the atom group. Solving issue #61 would allow to solve this easily, too.
Meanwhile, a
f_cvc_fit_gradients
feature could be used to print a Warning message.The text was updated successfully, but these errors were encountered: