Skip to content

SimStore: Periodic CVs#990

Merged
dwhswenson merged 7 commits into
openpathsampling:masterfrom
dwhswenson:periodic_cvs
Mar 14, 2021
Merged

SimStore: Periodic CVs#990
dwhswenson merged 7 commits into
openpathsampling:masterfrom
dwhswenson:periodic_cvs

Conversation

@dwhswenson

Copy link
Copy Markdown
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

codecov Bot commented Mar 11, 2021

Copy link
Copy Markdown

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

@sroet sroet left a comment

Copy link
Copy Markdown
Member

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.

Comment thread openpathsampling/experimental/simstore/test_storable_function.py Outdated
@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