-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add example of stitching NAGL charges into ff14SB + Sage #7
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great start and it ran successfully on my machine. But it should also have charge normalization to handle tricky cases.
nagl-biopolymer/run.py
Outdated
|
||
print("replacing dummy charges with NAGL charges ... ") | ||
for key, charge in interchange["Electrostatics"].charges.items(): | ||
if charge.m == 0.0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(not blocking) It'd be better if this were handled by checking the parameter ID instead of the numerical value. I'm not sure whether there are any 0-charge atoms in ff14SB but if we extend this workflow to other FFs this may become important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch, the housekeeping here was difficult for me to think through at first
), | ||
) | ||
|
||
print(f"total system charge is {get_total_charge(simulation.system)}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(blocking) We'll need to be able to handle really weird PTMs that withdraw/donate partial charge from/to neighboring residues. So some form of normalization will be needed in this workflow, but it should never change atoms whose charge was assigned by ff14sb. A minimal approach would be to normalize any nonintegral charges over all NAGL-assigned atoms. And an improvement on that would be to normalize within contiguous groups of NAGL-assigned atoms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough; in the spirit of "anything goes" I'm going with the simple approach. I think a more thorough charge normalization routine would require a decent amount of application-specific details that I don't want to implement here without some more consideration; it seems like the sort of feature that would be broadly useful to users of the toolkit who care about biopolymers so I'd like to see it upstreamed
With a really quick 1 minute simulation (9.3 ps) run locally, the simulation doesn't immediately unravel