Fix MISTIS minus ensemble not forbidding other states#983
Conversation
Codecov Report
@@ Coverage Diff @@
## master #983 +/- ##
==========================================
+ Coverage 80.26% 80.28% +0.01%
==========================================
Files 136 136
Lines 14455 14468 +13
==========================================
+ Hits 11603 11616 +13
Misses 2852 2852
Continue to review full report at Codecov.
|
|
Side note: looks like #980 did fix my issues with CodeCov. Now we get the comment after the last of the pytest runs has come in, but while the notebook tests are still running. Perfect! |
|
This is ready for review and comment. I will leave it open for at least 24 hours, merging no earlier than Thu 25 Feb 15:00 GMT (16:00 local). |
sroet
left a comment
There was a problem hiding this comment.
This LGTM, thank you for adding the understandable regression test. 1 extra newline snuck in, but don't block this PR based on that
Co-authored-by: Sander Roet <sanderroet@hotmail.com>
The minus interface as set up by the
MISTISNetworkdidn't forbid the trajectory from entering other states during the excursion from the state. This would occasionally manifest as a failure of the sanity check when saving, stating that the trajectory for some replica did not satisfy the ensemble (because the trajectory in the TIS ensemble after a replica exchange had frames in the other state).Typically, this meant that the minus trajectory had to cross to the other state and then cross back, so it was extremely rare, but I found it when working on a simple model that isn't actually rare enough to really need TIS to get the rate. @gyorgy-hantal: I think you encountered this problem once. This is the explanation.
This fixes it -- everything was in place to forbid other states (and used in MSTIS); just needed to add it to MISTIS. Also adds a test to prevent regression.