-
Notifications
You must be signed in to change notification settings - Fork 35
Change bonded host strategy to use lowest variance restraint #1477
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❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1477 +/- ##
==========================================
- Coverage 95.18% 92.37% -2.82%
==========================================
Files 165 165
Lines 12575 12640 +65
==========================================
- Hits 11970 11676 -294
- Misses 605 964 +359
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:
|
|
|
||
| bond_variance = np.var(restraints_eval.results.bonds[ridx]) | ||
|
|
||
| valid_variances.append(dih_variance + ang_variance + bond_variance) |
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.
Only using a linear combination of variances here. In the past, this "worked", but probably just as well as any other approaches. Happy to change it to something else if there's a better combination we can try.
hannahbaumann
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.
Thanks @IAlibay , lgtm, just the one thing. I tested this on one trajectory and it gave reasonable atoms. It does, however, take quite some time.
Do you have a ballpark timing @hannahbaumann ? i.e. is it order minute or tens of minutes or closer to hour? This being quite expensive is somewhat expected, the number of restraints it searches through pretty much explodes with the number of potential host anchor atoms it can find. How large was the max cutoff distance for host atoms? You'd want to keep that quite small, probably no larger than 1.5 nm. |
|
More like 30sec, so actually not that bad! |
O yeah, I was really expecting up to 10s of minutes! I think the search space grows by ~ 10-12 potential restraints per residue (if only doing backbone atoms), so it gets to pretty large numbers quickly if we're not careful. |
|
No API break detected ✅ |
hannahbaumann
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.
Thanks @IAlibay , lgtm!
Fixes #1471
Developers certificate of origin