Apply image shifting by fragment#124
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #124 +/- ##
==========================================
+ Coverage 98.26% 98.27% +0.01%
==========================================
Files 9 9
Lines 461 465 +4
==========================================
+ Hits 453 457 +4
Misses 8 8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| @@ -58,11 +58,15 @@ def apply_complex_alignment_transformations( | |||
| # 1. Make molecules whole (protein + optional ligand) | |||
| transforms = [unwrap(group)] | |||
There was a problem hiding this comment.
This now makes the alignment very very slow. Unwrap loops over the fragments and before (when there was no bond information), there were a lot of fragments that were very small.
There was a problem hiding this comment.
How much of a problem is this in practice?
IAlibay
left a comment
There was a problem hiding this comment.
One thing to fix, one question, and a missing news entry.
Otherwise it looks great to me!
| @@ -58,11 +58,15 @@ def apply_complex_alignment_transformations( | |||
| # 1. Make molecules whole (protein + optional ligand) | |||
| transforms = [unwrap(group)] | |||
There was a problem hiding this comment.
How much of a problem is this in practice?
| chains = [seg.atoms for seg in protein.segments] | ||
| shift_targets = chains[1:] + ligands | ||
| # 2. Closest image shift for protein fragments + ligand (if present) | ||
| fragments = list(protein.fragments) |
There was a problem hiding this comment.
Is it worth checking for bonds in protein before you do this?
| skip = max(n_frames // 500, 1) | ||
|
|
||
| u_top = mda.Universe(pdb_topology) | ||
| u_top.select_atoms("protein").guess_bonds() |
There was a problem hiding this comment.
I believe you have to avoid doing this if there is no protein, otherwise it'll crash out.
| u_top.select_atoms("protein").guess_bonds() | |
| protein = u_top.select_atoms("protein") | |
| if protein: | |
| protein.guess_bonds() |
Note: this also shows that we don't have any tests that are solvent only - we should raise an issue to deal with that.
When two chains of a protein have the same
segid, the PBC are not handled correctly during alignment since they would appear as one segment. This PR switches to use theprotein.fragmentsinstead. For fragments to be recognized correctly, we first need to guess the bonds in the protein.Having the large fragments now makes
unwrap()very slow which is fragment based.