Skip to content
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

MDAnalysis returns first letter of atom name as element #2313

Open
lilyminium opened this issue Aug 5, 2019 · 2 comments

Comments

@lilyminium
Copy link

commented Aug 5, 2019

Expected behavior

This is an excerpt of my real PDB file for ATP:

ATOM     20 AH2* ATP  1193      71.850  70.750  36.080  1.00  0.00            
ATOM     21 AC3* ATP  1193      69.340  70.550  35.970  1.00  0.00            
ATOM     22 AO3* ATP  1193      69.710  71.250  37.170  1.00  0.00            
ATOM     23 AH3* ATP  1193      69.260  72.140  37.120  1.00  0.00            
ATOM     24 AC5* ATP  1193      67.150  69.580  34.970  1.00  0.00            
ATOM     25 AO5* ATP  1193      66.640  70.800  34.400  1.00  0.00    

I think these atoms should have atom type/elements H, C, O, H, C, O.

Actual behavior

MDAnalysis thinks that all these atom types are 'A' with mass 0.0 (presumably because A is not an element).

Code to reproduce the behavior

Show us how to reproduce the failiure. If you can, use trajectory files from the test data.

import MDAnalysis as mda

u = mda.Universe('my_pdb_file.pdb')

print(u.atoms.types)

Currently version of MDAnalysis

  • Which version are you using? 0.19.3-dev
  • Which version of Python (python -V)? Python 3.7.3
  • Which operating system? MacOS Mojave 10.14.5

@lilyminium lilyminium referenced a pull request that will close this issue Aug 5, 2019

Open

Guess element #2314

3 of 4 tasks complete
@richardjgowers

This comment has been minimized.

Copy link
Member

commented Aug 5, 2019

Hi @lilyminium

This is because of this code here:

https://github.com/MDAnalysis/mdanalysis/blob/develop/package/MDAnalysis/topology/guessers.py#L109

Which isn't the best guesser imaginable. I think especially because it should always verify that what it returns is an element...

@lilyminium

This comment has been minimized.

Copy link
Author

commented Aug 6, 2019

Hi @richardjgowers

I'm not too up-to-date on GitHub etiquette, so perhaps this comment would be better suited in the pull request, but --

I did put together a patchy version at guessing cases like the above in #2314 . I would have thought that elements would only be the symbols, but MDAnalysisTests has a special test requiring that guess_atom_element('1') returns '1'. I haven't looked through the code to see if this functionality is used anywhere -- would you know if any file formats require elements to be numbers?

Currently the pull request checks atom names composed of letters for elements up to Calcium and returns anything that is composed entirely of digits and symbols. I figured I could optimise it for one task or another if the specification was clearer -- for now I just wanted to read in ATP :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.