Closed shuail closed 6 years ago
For the record, this was implemented by #18
@shuail - can you remind me the units of this threshold? Is this a score threshold, or a charge threshold? That is, is it in units of electrons, or an exponential of some factor?
The main concern I would have here would just be that if you have molecules which have slightly non-integer charge due to rounding, you can potentially get a contribution to the relative binding free energy by allowing the charge to change. Specifically, imagine the charging free energy of an ion is 100 kcal/mol, and you are mutating ligand A, with a charge of 0, into ligand B, with a charge of 1e-2 electrons (due to rounding error). To a first approximation we can assume linear response in water, but the binding site might be much less polar and less responsive. So for a worst case scenario let's assume that a change in charge of 1e-2 makes no difference in the binding site, but it gives a linear difference in water. So the 1e-2 change contributes (1e-2 electrons)*100 kcal/(mol electron) in water but 0 in the binding site, so your total relative binding free energy ends up with a contribution of 1 kcal/mol due to rounding error in your partial charges.
I think originally I chose a rather small threshold to make sure that this error would be very small even in such cases -- basically, I make sure the charges in my mol2 files are VERY precise so that rounding error in the charges remains smaller than 1e-3. I also went so far as to get OpenEye to adjust the precision used for charges in their mol2 files for exactly this reason: A small rounding error on charges for free energy calculations could introduce significant errors.
@davidlmobley The units of this threshold is in the units of electrons. And I have an example here to explain this issue.
Below are the molecules:
Molecule 1:
@
@
Molecule 2
@
@
Yes, I think the charge similarity is affected by the precision of the partial charges. For molecule 1, the total charge (summing of the partial charges) is 0.001 and for molecule 2, the total charge is -0.001. So the difference of the total charge is 0.002. If the threshold is 1-e3, then these two molecules have different charges and if the threshold is 1-e2, they will have the same total charges. While in this case, I think they all have zero total charges so that's why I choose to use the 1-e2 threshold.
Yes, I think the charge similarity is affected by the precision of the partial charges. For molecule 1, the total charge (summing of the partial charges) is 0.001 and for molecule 2, the total charge is -0.001. So the difference of the total charge is 0.002. If the threshold is 1-e3, then these two molecules have different charges and if the threshold is 1-e2, they will have the same total charges. While in this case, I think they all have zero total charges so that's why I choose to use the 1-e2 threshold.
@shuail - yes, this is exactly the issue I was describing. Note the charges on your atoms are all rounded to just three significant digits of precision or less (e.g. -0.117200
or 0.083000
or 0.058100
) though far more digits of precision are available. This naturally means you'll accumulate significant rounding error as you sum the charges, which:
a) results in the behavior you noted where the net charge may not quite be integer so you need to adjust the threshold for the mapper
b) ALSO can cause the behavior I described where the same issue could introduce errors in computed free energy differences
I'd suggest that you instead add additional precision for your charges.
There is actually utility functionality in ParmEd that can do this for you -- it provides a "fix_charges" function which can make your charges sum to an integer. :) See https://github.com/ParmEd/ParmEd/issues/268 and https://github.com/ParmEd/ParmEd/pull/271.
So, I want to emphasize: a better fix is NOT to adjust the threshold for charge similarity, but to make sure your charges sum to an integer. The former is just papering over the problem and could introduce error; the latter avoids the threshold problem AND the error.
@davidlmobley I see the point now, yes, the sum of partial charges is not zero in these molecules. I think for this dataset, I just download the mol2 files instead of generating from smile strings myself. So it probably has bad precision. I agreed that the total charge should be integer for input mol2 files for running the simulations. While for the original code with 1-e3 threshold, when I generate the graph, it's hard to distinguish cases with different total charge in 1 electron unit or in 0.002 electron unit. So should we modify the code to report a Warning of the ligands with non-integer total charge?
@shuail :
I think for this dataset, I just download the mol2 files instead of generating from smile strings myself. So it probably has bad precision.
It's not just that -- it's fairly common to have mol2 files which have exactly this issue (partly because we don't think we KNOW partial charges that precisely, so we usually round to not very much precision). Normally we think this is unimportant, but this is one case where it IS important for the reasons I described.
The best solution is just to make sure that you process molecules before feeding them in to ensure that they have charges very close to integer.
I agreed that the total charge should be integer for input mol2 files for running the simulations
Note they won't necessarily be quite integer because of rounding. The issue is how close to integer you are -- you need to be "close enough", which is what this tolerance is about.
I guess what I'm saying is that there are two issues: 1) You should pre-process your mol2 files to ensure you have basically integer charges to a fairly high level of precision (which the code I linked to earlier can do for you) 2) You should set this threshold sufficiently small (probably 1e-3) that it will never introduce significant error
So should we modify the code to report a Warning of the ligands with non-integer total charge?
Probably, yes. You probably want it to tell you that you should be pre-processing your ligands to ensure the charges are within tolerance of integer. :)
Sounds reasonable to me. I will modify the code back into 1-e3 threshold for detecting different total charges and add Warnings if the individual total charge is not close enough to integer by 1-e3 threshold.
I think this was resolved by #28 ? Were any others?
@davidlmobley this issue is not resolved by #28 , create a new pull request #29 to address this.
Resolved by #29 .
I loose the total charge comparison threshold from 1e-3 to 1e-2 and commit to shuaidev branch. It improves our test cases performance (put molecules which we thought have identical net charge into the same group) while more validation may needed. If this loose threshold cause any problems, we could discuss here or change to a net charge comparison instead of summing all partial charges which we are using in the code now?