-
Notifications
You must be signed in to change notification settings - Fork 30
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
Update openeye.get_charges to allow retaining of provided conformer #146
Comments
@hanhnguyen14 - this is the issue you brought up. |
So afaik the current default is based on best practices according to chris
|
@kyleabeauchamp - I think you misunderstand the issue. The current default indeed implements Bayly's best practices for charge calculation, and I don't propose changing how the charges are calculated. What I propose changing is what conformer is returned. That is, I ought to be able to get back the charges applied to my existing conformer if I want. Imagine, for example, that I pull a PDB ligand conformation, calculate charges on the ligand, and then use it to run a simulation of the ligand on the binding site - expecting that I'll be simulating the ligand starting with the current bound conformation. Currently, this in fact would not be what I get because the charge calculation procedure actually changes the conformation. The expected behavior would naively be that the charge calculation procedure just calculates charges (via Bayly's best practices) and then applies those charges to whatever conformation I've provided (i.e. my ligand conformation from the PDB). So the issue is just which conformation is returned, NOT how the charges are obtained. |
@davidlmobley: This is the official, textbook, 100% Christopher Bayly recommended method for "Generating Canonical AM1-BCC Charges" scheme direct from his OpenEye Toolkit Cookbook article: We have been assured that the conformation generation scheme is integral to producing the correct charges in this scheme. The scheme you describe, while seemingly sensible, is not the "Canonical AM1-BCC Charge" scheme we should be recommending to users. |
@jchodera , @kyleabeauchamp : You guys are STILL missing my point. I am NOT talking about changing the charge calculation recipe. Bayly's recipe refers to how to calculate charges, and I 100% agree with it. HOWEVER, the current function not only calculates charges via Bayly's recipe, it ALSO modifies the conformation of the molecule which was provided, which is an entirely separate issue which is not part of his recipe. Let me summarize my point by comparing the two implementations in question: Current implementation: Proposed implementation (with an option to still use the above implementation): Again, I am not proposing changing the conformation generation scheme, NOR the recipe for calculating charges. I am only proposing changing the aspect wherein the user-supplied conformation is discarded whether they want it or not. It absolutely should not be used in the charge calculation, but the user might want to continue subsequent work from this conformation! The main reason this is an issue is, as I said, that I may want to begin a simulation from a specific conformation of a molecule (as Chris Bayly himself does frequently). I should be able to calculate charges via Bayly's recipe and APPLY them to my desired conformation. To put it one final way - a charge calculation function ought to calculate charges and assign them to my molecule. It ought not to modify the conformation of my molecule, unless I ask it to. ABSOLUTELY it needs to follow Bayly's recipe for GENERATING the charges (and this includes conformer generation). But the user doesn't need his INPUT conformation modified. |
D'oh. I somehow missed that the "unintended side effect" was "modifying the input molecule". This is an easy fix. Hold on for a PR. |
Yeah, I was going to do it myself shortly (already have the code, just dealing with some travel issues). |
OK, let me know if you want me to do it. The current argument
I think we want to
|
I'm just not sure if changing the default behavior will break any of @kyleabeauchamp's code. |
My initial plan was to just make it do the first of those, and break the old code (since @kyleabeauchamp has mostly seemed OK with that for these types of issues) if it came to that. I guess if we desperately want to do it without breaking the old code, we can return 801 conformations if I suppose if we want to do it without breaking old code, we probably ought to add a third option, |
I vote for breaking backward-compatibility here unless @kyleabeauchamp objects. |
No objections On Mon, Jul 13, 2015 at 5:14 PM, John Chodera notifications@github.com
|
Currently, the
openeye.get_charges
function has an undocumented "feature" in that it generates entirely new conformations for the provided molecule and returns those along with the new charges (openmoltools/openmoltools/openeye.py
Line 13 in 9cf5f61
This is often fine for solution-phase calculations where conformational interconversion times are short, but if we are looking at something like protein-ligand binding where we might want to start with the ligand already placed into the binding site, this is not correct.
The
openeye.get_charges
function needs to be updated so that:a) the default behavior is to calculate charges as it currently does, and copy these back onto the provided conformation
b) optionally it can provide the calculated charges on entirely new conformations (as at present)
The text was updated successfully, but these errors were encountered: