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

Removing false positive detection of H-atoms #33

Closed
wants to merge 3 commits into from

Conversation

arghya90
Copy link

@arghya90 arghya90 commented Nov 8, 2017

In the function 'freesasa_pdb_ishydrogen', the method of detecting h-atoms resulted into some non-H atoms being discarded. For example, oxygen atoms (OD, OD1, etc.) and carbon atoms (CD, CD1, etc.). I have added an extra line of if-statement that corrects for that.

Kindly consider.

Arghya Chakravorty added 3 commits November 8, 2017 11:25
This function previously identified some non-hydrogen atoms as hydrogens if the second character in the atom name was 'H/D'. Example: CD,CD1,CD2 or OD1, OD2 etc. Due to this the number of non-H atoms being read were incorrect. 

A new line in the function has been added (defined in pdb.c)
This function previously identified some non-hydrogen atoms as hydrogens if the second character in the atom name was 'H/D'. Example: CD,CD1,CD2 or OD1, OD2 etc. Due to this the number of non-H atoms being read were incorrect. 

A new line in the function has been added (defined in pdb.c)
Improved 'freesasa_pdb_ishydrogen' function
@mittinatten
Copy link
Owner

Hi and thank you! I'm not sure this does what you intended it to do, it causes a bunch of errors in the automated tests, and the code looks like it will miss a lot of true positives.

Do you have a sample file, or even line from a pdb file, that fails?

@arghya90
Copy link
Author

arghya90 commented Nov 8, 2017 via email

@mittinatten
Copy link
Owner

mittinatten commented Nov 8, 2017

Hi,
seems the attachments didn't make it through to Github. If you could just insert a few of the offending lines here in the thread I will have something to work with (paste it as code to get the whitespace right).

This very basic test fails: https://github.com/mittinatten/freesasa/blob/master/tests/test_pdb.c#L87 in your code. The other errors seem to be that for the PDB files in the test suite with hydrogens, hydrogens are not skipped when they're supposed to and you get the wrong number of atoms. If you try 1D3Z with your code for example (can be found in tests/data/), I think you will get a bunch of warnings.

@mittinatten
Copy link
Owner

Closing this for now. From what you describe it sounds like you might have been using non-conforming PDB files rather than there being a bug. Feel free to reopen if there's any further information or development.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants