-
Notifications
You must be signed in to change notification settings - Fork 49
SimStore: Periodic CVs #990
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
@@ Coverage Diff @@
## master #990 +/- ##
=======================================
Coverage 80.58% 80.58%
=======================================
Files 138 138
Lines 14671 14671
=======================================
Hits 11823 11823
Misses 2848 2848 Continue to review full report at Codecov.
|
|
This is ready for review and comment. I will leave it open for at least 24 hours, merging no earlier than Sun 14 Mar 19:00 GMT (20:00 local). |
sroet
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 in general. I would like a slightly more specific test (just to make sure the error that you expect is raised). But please don't hang this PR on that.
openpathsampling/experimental/simstore/test_storable_function.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Sander Roet <sanderroet@hotmail.com>
This PR adds information about periodicity to SimStore CVs. It reserves the kwargs
period_minandperiod_max.This isn't currently used anywhere in the core of OPS, but might be used in the future to combine
PeriodicCVDefinedVolumeandCVDefinedVolume(probably a good option for OPS 2.0).It will be used in the CLI for simulation setup.