Skip to content

Fix relative tolerance #604

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Mar 3, 2025
Merged

Fix relative tolerance #604

merged 9 commits into from
Mar 3, 2025

Conversation

lilyminium
Copy link
Contributor

Description

Fixes #592

Changes:

  • changes which uncertainty is compared
  • Adds tests of directly executing a Simulation graph, as well as with a server
  • Adds test data from an example run
  • Adds some helper functions to tests

Blocked by / follows #602.

Questions

  • I've started adding test data from example runs -- this is helpful firstly in having an example of test data, and secondly for avoiding re-execution of workflows during tests. However there are a lot of files -- should these be compressed and uncompressed on-the-fly? Pros for compression are that the number of files changed is much lower. Cons is that the tarball may wind up being quite large (10+MB), and any changes to an individual file will change the entire compressed directory.

Status

  • Ready to go

@codecov Codecov
Copy link

codecov bot commented Jan 21, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.27%. Comparing base (7d927d9) to head (0d342d8).
Report is 18 commits behind head on main.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lilyminium lilyminium mentioned this pull request Jan 23, 2025
Merged
1 task
mattwthompson
mattwthompson approved these changes Feb 13, 2025
View reviewed changes
Copy link
Member

@mattwthompson mattwthompson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mild preference for compressing data files since they're already so large, but I think we should go with whatever option slows you down the least

@lilyminium
Copy link
Contributor Author

Thanks Matt! I'll merge this in for now. I'm about to add a whole lot more test data to some of my other PRs so we might want to revisit better test data storage solutions there, but let's see how bad it is first :-)

@lilyminium lilyminium merged commit c5c5292 into main Mar 3, 2025
14 checks passed
lilyminium added a commit that referenced this pull request Mar 3, 2025
@lilyminium @pre-commit-ci @mattwthompson
Fix relative tolerance (#604)
bb6ca1c 
* add nobatch

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* add fix

* add helper test functions

* add test data

* Revert "add fix"

This reverts commit 3d9f043.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Revert "Revert "add fix""

This reverts commit 79a8bea.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Matthew W. Thompson <[email protected]>
lilyminium added a commit that referenced this pull request Mar 25, 2025
@lilyminium @pre-commit-ci @mattwthompson
Add Equilibration Layer (#608)
ac87073 
* add equilibration code

* add some preliminary docs

* add some docstrings

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* switch greaterthan -> greaterthanorequalto

* add updates for equilibrated properties

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix equilibration_properties

* Add easy way to create new substances for real numbers of mols (#611)

* add easy way to create new substances

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Add new conditions and tests (#614)

* add new conditions and tests

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Add NoBatch mode (#602)

* add nobatch

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* add test

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Migrate `tmpdir` fixture to new `tmp_path` (#615)

* [pre-commit.ci] pre-commit autoupdate (#617)

* [pre-commit.ci] pre-commit autoupdate

updates:
- [github.com/psf/black: 24.10.0 → 25.1.0](psf/black@24.10.0...25.1.0)
- [github.com/PyCQA/isort: 5.13.2 → 6.0.0](PyCQA/isort@5.13.2...6.0.0)

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Add AttributeClass initialization (#606)

* add failing test

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* add __init__ method

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Strip out `openmm.CMMotionRemover` force internally (#622)

* Automatically strip out `openmm.CMMotionRemover` force internally

* More consistently strip force

* Fix relative tolerance (#604)

* add nobatch

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* add fix

* add helper test functions

* add test data

* Revert "add fix"

This reverts commit 3d9f043.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Revert "Revert "add fix""

This reverts commit 79a8bea.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Matthew W. Thompson <[email protected]>

* allow boxes to be retrieved, read, and short-circuit the graph

* add data

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* remove additional raise

* disallow merging

* rm doc stub for now

* Update openff/evaluator/workflow/workflow.py

* Update openff/evaluator/properties/density.py

* remove unused imports

* fix imports

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Matt Thompson <[email protected]>
Co-authored-by: Matthew W. Thompson <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattwthompson mattwthompson mattwthompson approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Relative uncertainty does not result in successful properties unless relative_uncertainty > 1.0
2 participants
@lilyminium @mattwthompson