UCL-CCS / TIES

Topology Superimposition based on joint graph traversal
MIT License
5 stars 1 forks source link

command line wrong alchemical indexes #304

Closed adw62 closed 2 years ago

adw62 commented 2 years ago

Hey,

Im still messing with this fluorine data set. I thought I could use the command line to by pass the api issues we are having with the spurious bond.

The command line is giving me incorrect results though and the alchemical indexes are incorrectly assigned, the behavior is also inconsistent with the API

This is my input and output: cmd_v_api.zip

the ties dir was created with the api using this cmd: ties create -l ligand.mol2 l_H18.mol2 -p receptor.pdb -nc -1 -pff leaprc.protein.ff14SB

The ./l_H18 dir is created by the API in Fluorine_Scan.ipynb

In the ties dir you will see the alchemical indexes are wrong: Screenshot from 2022-07-20 11-18-54

and one of the carbons is appearing when it should be disappearing (note the spurious bond is gone) the incorrect indexes happen for all my input ligands not just H18

The indexes are correct in ./l_H18 but of course the spurious bond is persisting

What is causing this inconsistent behaviors between API an command line?

bieniekmateusz commented 2 years ago

It is kind of the case that #302 fixes it (as in, with -uniq-a true the -1 and 1 inside of the complex.pdb file are correct)

So previously in MDAnalysis you would do A1 == A2 and if the atoms were "the same" as in the same atom names (cannot remember other details) than it would return True, even if the atom objects were from different files.

This is different in Parmed, were the A1 == A2 now returns False instead.

I'll check now how it affects the software.

bieniekmateusz commented 2 years ago

I confirm that it is to do with renaming. When I use API and I apply pair.make_atom_names_unique() then I get two atoms disappears and two atoms appearing, which is correct. However, if I don't I get the 3 atoms appearing and 1 disappearing. I'll check where the equivalency tests are used.

adw62 commented 2 years ago

Okay nice thanks for all the fixes. BTW we did a run through of TIES20 and TIESMD with a visitor, using the API everything worked great, I'll give the command line a test tomorrow.

The only thing that tripped us up was in using NAMD. TIES20 seems to rename all ATOMS in the PDB to HETERATOMS and NAMD does not like this. Any way to change this? I'll also have a look at this Monday I think I should be able to fix it myself.

bieniekmateusz commented 2 years ago

Just for the record. This error must have been there since always but I always renamed atoms I guess. Apparently I was using atom names during the writing of the PDB file and so if they were not unique, something odd would come out.

However, this seems to be a deeper flow in using the atom names (obviously). I'll create a separate issue for that. For now, let's ensure all atoms are unique.

bieniekmateusz commented 2 years ago

With the HETERATOMS I am not sure how that could happen.

There is a method write_pdb() in SuperimposedTopology with something like this:

line = f"ATOM  {atom.idx:>5d} {atom.name:>4s} {atom.residue.name:>3s}  " \
                           f"{1:>4d}    " \
                           f"{atom.xx:>8.3f}{atom.xy:>8.3f}{atom.xz:>8.3f}" \
                           f"{1.0:>6.2f}{REMAINS:>6.2f}" + (' ' * 11) + \
                           '  ' + '  ' + '\n'

I am happy to have a look at the file.

adw62 commented 2 years ago

Can we run pair.make_atom_names_unique() as part of the command line execution, maybe this is the easier and most safe solution, also possibly what you are already suggesting?

I'll get the files for the HETROATOM thing, have a play, and if I can fix it I'll make a new issue with the files.

bieniekmateusz commented 2 years ago

Sure! I'll submit it now.