Skip to content

Conversation

@dwhswenson
Copy link
Member

This PR adds information about periodicity to SimStore CVs. It reserves the kwargs period_min and period_max.

This isn't currently used anywhere in the core of OPS, but might be used in the future to combine PeriodicCVDefinedVolume and CVDefinedVolume (probably a good option for OPS 2.0).

It will be used in the CLI for simulation setup.

@codecov
Copy link

codecov bot commented Mar 11, 2021

Codecov Report

Merging #990 (4377970) into master (e48262e) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           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.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e48262e...4377970. Read the comment docs.

@dwhswenson
Copy link
Member Author

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).

Copy link
Member

@sroet sroet left a 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.

@dwhswenson dwhswenson merged commit d303ff6 into openpathsampling:master Mar 14, 2021
@dwhswenson dwhswenson deleted the periodic_cvs branch March 14, 2021 21:05
@dwhswenson dwhswenson mentioned this pull request Jul 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants