-
Notifications
You must be signed in to change notification settings - Fork 35
Small FF assignment refactor of the AFE base unit class #1289
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1289 +/- ##
=======================================
Coverage 92.73% 92.73%
=======================================
Files 141 141
Lines 10860 10860
=======================================
Hits 10071 10071
Misses 789 789
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
No API break detected ✅ |
atravitz
left a comment
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.
lgtm! just a typo
| system : openmm.System | ||
| An OpenMM System of the alchemical system. | ||
| positionns : openmm.unit.Quantity | ||
| An non-alchemical OpenMM System of the simulated 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.
| An non-alchemical OpenMM System of the simulated system. | |
| A non-alchemical OpenMM System of the simulated 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.
Oops auto-merge did itself before I got to merge this.
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.
Will add it in the next PR.
I'm cherry picking small changes to the ABFE PR to make it a lot easier to deal with / work out what remains to be tested.
This change purely affects how the base unit defines how we fetch OpenMM topology/system objects. It's mostly a convenience update but makes downstream classes a lot simpler.
Checklist
newsentry -- this is an internal API thing, no news is required.Developers certificate of origin