Atomic properties and adjacency matrix for each target mol are used#155
Atomic properties and adjacency matrix for each target mol are used#155DaniDelHoyo wants to merge 4 commits intoRMeli:developfrom
Conversation
|
Hi @DaniDelHoyo, thank you for rising the problem and for your contribution! The fact that all molecules have the same atomic properties and adjacency matrix in However, I understand this might be an issue if it fails with incorrect results. Could you please share a bit more about the calculation that was failing for you (i.e. a simple reproducer), so that we can decide how to move forward? |
|
Hi @RMeli , thank you for your answer. I did notice there was some failing tests sorry, I didn't have the time to check them. When executing spyrmsd such as: However, if we skip the mol2.sdf (and therefore, the adjacency matrix of crystal.pdb is used in the RMSD calculation), it's RMSD is quite different: I have noticed the same behavior in some other cases but this was the clearest. |
|
I have just noticed that my changes would only make an effect if the parameter "cache" is deactivated (True by default), because with cache=True, the isomorphism from the first molecule will be used even if the whole list of adj. matrix and atom properties are passed. So it would be necessary to control this cache parameter too, somehow. |
Description
This PR is meant to solve an issue when calculating the RMSD on a list of target molecules. At least when using the command line, def symmrmsd iterates over the target molecules coordinates and send them to _rmsd_isomorphic_core for the calculation.
However, the atomic properties and adjacency matrix sent is the one of the first target molecule (lines 369 and 371):
mols[0].atomicnums
mols[0].adjacency_matrix
This assumes that the properties and adjacency matrix of all the target molecules are all the same, which was not my case, and leads to incorrect RMSD calculations. As we have that information for each of the molecules, it is not difficult to just pass the respective atomic properties and adjacency matrix for each of the molecules.
I have not tested in depth if my changes break anything else though, so please take it into account first.
Checklist