-
Notifications
You must be signed in to change notification settings - Fork 23
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
Handle some edge cases with preset charges #1070
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
c205ffc
to
4099a1e
Compare
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.
These are great improvements I think. I think the documentation changes from #1048 could be merged with this so that the charge hierarchy is only documented once, which might improve findability - I think if there is similar documentation in two places, people sometimes read one place carefully, then later find the second place and think they're already familiar with it coz it looks the same.
My notes are mostly just adding more detail to what you've written, but this is great!
Co-authored-by: Josh A. Mitchell <yoshanuikabundi@gmail.com>
Co-authored-by: Josh A. Mitchell <yoshanuikabundi@gmail.com>
Co-authored-by: Josh A. Mitchell <yoshanuikabundi@gmail.com>
Co-authored-by: Josh A. Mitchell <yoshanuikabundi@gmail.com>
Co-authored-by: Josh A. Mitchell <yoshanuikabundi@gmail.com>
Co-authored-by: Josh A. Mitchell <yoshanuikabundi@gmail.com>
Co-authored-by: Josh A. Mitchell <yoshanuikabundi@gmail.com>
for more information, see https://pre-commit.ci
Aiming to sync this up and re-visit, probably to finish it up, after #1053 is merged in |
Thank you @Yoshanuikabundi! |
Description
This PR better handles user inputs around the
charge_from_molecules
argument from the toolkit. Resolves #1057 #1058 #1059Checklist
charge_from_molecules
contains any molecule without partial chargescharge_from_molecules
contains any duplicate molecules (defined by isomorphism, approximated by SMILES without hydrogens)charge_from_molecules
on virtual sites / charge increments can be unexpected. #1050from_smirnoff
docstringAdd FAQ entryI guess we don't have an FAQ here, maybe should checkopenff-docs