-
Notifications
You must be signed in to change notification settings - Fork 64
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
Rewrite and improve DSSP #748
Conversation
…finitions of Kabsch and Sander
…d idx to refer to things indexed in Residues_
…ctive for comparison. Start doing strand assignment.
…aracter assignment still seems off
…a hbond network known
…ns are possible. When looking for beta, do not attempt to assign if 3-residue stretches would overlap. Report bend as degrees in debug.
…end, only assign one segment at a time.
… priority assignment not already present
This pull request introduces 1 alert and fixes 1 when merging f9a0179 into de38e93 - view on LGTM.com new alerts:
fixed alerts:
Warning - Automated code review for Amber-MD/cpptraj will be disabled on October 1, 2019. You can avoid this by installing the LGTM.com GitHub App. Read about the benefits of migrating to GitHub Apps in the blog. Comment posted by LGTM.com |
This pull request introduces 1 alert and fixes 1 when merging 33a231e into de38e93 - view on LGTM.com new alerts:
fixed alerts:
Warning - Automated code review for Amber-MD/cpptraj will be disabled on October 1, 2019. You can avoid this by installing the LGTM.com GitHub App. Read about the benefits of migrating to GitHub Apps in the blog. Comment posted by LGTM.com |
The Travis failure is very strange:
What's strange is that the same apt-get command worked for other builds, so maybe this is a temporary blip. The Jenkins failure are from PGI compilers 18.10. I'll try to get my hands on that to check out what might be going on - I suspect it's mathematical differences based on the tests that fail (curve fitting etc). |
This pull request fixes 1 alert when merging f936521 into de38e93 - view on LGTM.com fixed alerts:
|
This pull request fixes 1 alert when merging 77d8eb0 into de38e93 - view on LGTM.com fixed alerts:
|
Remaining failures are related to PGI compilers 18.10 (and DSSP test cases are passing with that). Will try to fix PGI stuff in a different PR. Since everything else is passing, I will merge. |
Started as a way to address #691 and ended up being a lot more.
This PR constitutes a pretty complete rewrite of the DSSP secondary structure assignment functionality. The assignment now matches the original paper exactly, i.e. extended beta and isolated beta bridge is assigned instead of parallel/anti-parallel beta. I had written it like this because that's the way it was originally in ptraj and I wanted to keep backwards compatibility, but I think it makes more sense to use the original definitions if for no other reason than a residue in a strand could be potentially both parallel and anti-parallel. The original behavior can be restored via a new keyword,
betadetail
.The new code is actually faster to assign secondary structure than the original code (although the bottleneck is still hydrogen bond calculation), and uses way less memory. The helix assignment code is much more robust as well, and isolated stretches which used to be labeled as a helix are now correctly labeled as turn. Also, beta bulge structure is now identified and correctly assigned extended in accordance with the original definition.