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

GO IDP bugfix #590

Closed
wants to merge 7 commits into from
Closed

GO IDP bugfix #590

wants to merge 7 commits into from

Conversation

csbrasnett
Copy link
Collaborator

Need to add secstruc to the water bias application, otherwise can't apply across a whole protein (starting resid has to be > 1). this should fix that. And add preprint citation.

@pckroon pckroon requested a review from fgrunewald April 23, 2024 13:37
Copy link
Member

@fgrunewald fgrunewald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs at least one integration test in my opinion.

@csbrasnett do you think the citation should also be added in the water bias processor?

@csbrasnett
Copy link
Collaborator Author

I'm not certain, but I think adding again there would be degenerate anyway? VirtualSiteCreator gets called both in the Go model and if args.water_bias is used, so I think just adding it there should be fine.

Happy to write an integration test. It strikes me this might be a good opportunity to deal with #585 at the same time, so I can do that as well?

@fgrunewald
Copy link
Member

@csbrasnett I'd rather have 2 separate PRs, so that we can release this bug fix in near time

Copy link
Member

@fgrunewald fgrunewald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good but waiting for @csbrasnett to complete all changes

with the extra flag -idp-tune (probably a better name) martinize
will add additional potentials as per the Go paper.

This commit:
1) finds cross Go interactions between folded and idrs and removes
them
2) identifies BB and SC1 beads in idrs
@csbrasnett
Copy link
Collaborator Author

This should bring things in line with polyply for generating topologies of idps. Maybe this is the right approach, maybe it isn't, but here's some kind of implementation either way. Happy to be advised whatever.

To do:

  1. properly check against polyply
  2. make some tests
  3. maybe propagate secondary structure for automatic id/suggestions of IDRs?

@csbrasnett csbrasnett closed this Apr 25, 2024
@csbrasnett csbrasnett deleted the idp-bugfix branch April 25, 2024 12:27
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