ReliaSolve / cctbx_project

Computational Crystallography Toolbox
https://cctbx.github.io
Other
0 stars 0 forks source link

Hydrogens added in 1brf that should not be #256

Open russell-taylor opened 1 year ago

russell-taylor commented 1 year ago

This file reports an irreducible clique of size 4 that has to be reduced using brute force. Reduce running on this file is very fast (fraction of a second) because it does not have any cliques, only singletons (even with -rad0.5 or -rad1.0). Reduce2 is many minutes (1909 seconds)

The residues are in chain a: Cys 5, Cys 8, Cys 38, Cys 41. They are all on Sulfurs that are near an Fe ion. Reduce adds no hydrogens here, presumably because it thinks that the Sulfurs are ionically bonded to the Iron.

russell-taylor commented 1 year ago

@todo: This also happens with A Ser 201 OG bonded to K in 1zz0 (which cannot complete optimization so we needed to add intermediate prints to find this out).

russell-taylor commented 1 year ago

Asked Dorothee to take a look at this and she said she would within a couple of weeks after 6/30/2023.

russell-taylor commented 1 year ago

Fe should be a positive ion, so it seems like there should not be hydrogen bonding with other nearby atoms that have hydrogens attached, otherwise this could be a candidate for https://github.com/ReliaSolve/cctbx_project/issues/264

russell-taylor commented 1 year ago

@todo: In mmtbx.hydrogen.reduce_hydrogen.py, #p.pdb_interpretation.automatic_linking.link_metals = True is commented out, which may have something to do with this.

Indeed, when this is put back in we no longer get these hydrogens added to the system. Adding this does not change the results for 1xso (we hope that it would not).

Note from Dorothee on why we don't put this back in to fix it: Hi Russell, Thanks for looking into this. Currently, metal linking is done in two places: automatic in pdb_interpretation, and there is an additional routine that is activated with this parameter. Unfortunately, that code is less elaborate than the automatic one. Our plan is to make metal linking more consistent and avoid doing it in two places (so ideally we will not need this keyword). We don't want to activate the parameter as a temporary fix, because then we may forget that this was an issue in the first place. Best wishes, Dorothee