openfe
openfe copied to clipboard
[WIP] Restraints API
Checklist
- [ ] Added a
newsentry
Developers certificate of origin
- [x] I certify that this contribution is covered by the MIT License here and the Developer Certificate of Origin at https://developercertificate.org/.
🚨 API breaking changes detected! 🚨
Codecov Report
:x: Patch coverage is 98.80869% with 17 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 93.20%. Comparing base (ccf856a) to head (27447b3).
:warning: Report is 226 commits behind head on main.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| ...protocols/restraint_utils/openmm/omm_restraints.py | 92.18% | 10 Missing :warning: |
| openfe/protocols/restraint_utils/geometry/utils.py | 96.55% | 7 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #1043 +/- ##
==========================================
- Coverage 94.63% 93.20% -1.44%
==========================================
Files 143 165 +22
Lines 10962 12389 +1427
==========================================
+ Hits 10374 11547 +1173
- Misses 588 842 +254
| Flag | Coverage Δ | |
|---|---|---|
| fast-tests | 93.20% <98.80%> (?) |
|
| slow-tests | ? |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
🚨 API breaking changes detected! 🚨
🚨 API breaking changes detected! 🚨
🚨 API breaking changes detected! 🚨
🚨 API breaking changes detected! 🚨
🚨 API breaking changes detected! 🚨
🚨 API breaking changes detected! 🚨
🚨 API breaking changes detected! 🚨
🚨 API breaking changes detected! 🚨
🚨 API breaking changes detected! 🚨
🚨 API breaking changes detected! 🚨
🚨 API breaking changes detected! 🚨
🚨 API breaking changes detected! 🚨
🚨 API breaking changes detected! 🚨
🚨 API breaking changes detected! 🚨
🚨 API breaking changes detected! 🚨
Does OpenMM have pbc wrapping issues with restraints, similar to gromacs? I just tested the code on T4-lysozyme and it selected protein atoms quite far from the ligand which would cause problems in Gromacs (if the distance between atoms is close to half the box size). But I'm not sure if OpenMM deals with this maybe better?
Does OpenMM have pbc wrapping issues with restraints, similar to gromacs? I just tested the code on T4-lysozyme and it selected protein atoms quite far from the ligand which would cause problems in Gromacs (if the distance between atoms is close to half the box size). But I'm not sure if OpenMM deals with this maybe better?
I'll check re: OpenMM. Regarding "residues picked far away", this looks to me like a consequence of the algorithm design. If we want to pick from nearby residues, we should tighten up the minimum distance criteria and sort the host atoms by their distance from the ligand.
The latter is what I do elsewhere and would be my recommendation, but I didn't want to mess with the implementation too much.
Yes, sorting by distance could make sense! I had this filter (an additional max_filter which I'm not sure if it fully makes sense...) that makes sure that the atoms (different protein atoms relative to each other and to the ligand atom l1), are not further apart than a little less than half the box size. https://github.com/MobleyLab/SeparatedTopologies/blob/e7f712379472ea320cc049a78b19405014d8bbce/boresch_restraints.py#L389-L390
I had this filter (an additional max_filter which I'm not sure if it fully makes sense...) that makes sure that the atoms (different protein atoms relative to each other and to the ligand atom l1), are not further apart than a little less than half the box size.
Unfortunately filtering by half box isn't as simple when you have a non cubic cell. This is why we instead implemented a "take the minimum combined distance between P3-P1 and P3-P1". Anyways, sorting by distance will yield similar outcomes since we sequentially iterate through the atom list.
No API break detected ✅
Dismissing @hannahbaumann's review as most things have either been addressed or will be addressed in a future PR.