Add Equilibration Layer by lilyminium · Pull Request #608 · openforcefield/openff-evaluator

lilyminium · 2025-01-21T05:08:41Z

Description

Fixes #607
Fixes #616
Fixes #609

This peels off code from #603.
It relies on additions in #602 and #606 to work.

Adds:

StoredEquilibrationData (for data storage)
EquilibrationDataQuery (for data retrieval)
EquilibrationProperty (for convergence)
EquilibrationSchema, EquilibrationLayer, base classes
additional convergence conditions for multi-property convergence and any/all specification
allowing UNDEFINED properties to store data

To do:

add tests for requiring a number of uncorrelated samples

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

* 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 * [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 * [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>

* [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 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>

* Automatically strip out `openmm.CMMotionRemover` force internally * More consistently strip force

* 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 <mattwthompson@protonmail.com>

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

codecov · 2025-03-04T01:14:11Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.59%. Comparing base (c5c5292) to head (f0b9322).
Report is 21 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 · 2025-03-12T05:00:27Z

Installing a new dev environment from the conda env file caused an equilibration on Slurm to hang for me. Changing the below helped:

dask 2024.2.1 -> 2024.12.1
installing OpenEye

I'm not sure which one of these resulted in the fix; maybe charge assignment was hanging? Just a note to self in case this should be looked into.

lilyminium · 2025-03-17T21:32:54Z

openff/evaluator/layers/layers.py

                if isinstance(data_object.force_field_id, PlaceholderValue):
                    data_object.force_field_id = batch.force_field_id
                if isinstance(data_object.source_calculation_id, PlaceholderValue):
                    data_object.source_calculation_id = batch.id
+                data_object.calculation_layer = layer_name


This has effects for previously stored simulation data, although hopefully not large ones.

lilyminium · 2025-03-17T21:33:40Z

openff/evaluator/layers/layers.py

-                if returned_output.physical_property == UNDEFINED:
+                if (
+                    returned_output.physical_property == UNDEFINED
+                    or returned_output.physical_property.value == UNDEFINED


I'm actually not sure the first condition in this if statement (the one originally present) would have ever triggered; for an empty result, it's the .value that's UNDEFINED. Nevertheless could have unforeseen effects

lilyminium · 2025-03-17T21:35:10Z

openff/evaluator/layers/workflow.py

+            if workflow_result.value != UNDEFINED:
+                physical_property.value = workflow_result.value.value
+                physical_property.uncertainty = workflow_result.value.error
+
+                if len(workflow_result.gradients) > 0:
+                    physical_property.gradients = workflow_result.gradients
+            else:
+                physical_property.value = UNDEFINED
+                physical_property.uncertainty = UNDEFINED


Uh, I think this was a real bug where physical properties that couldn't be computed would just run into an error here.

That seems likely to me!

lilyminium · 2025-03-17T21:36:15Z

openff/evaluator/layers/layers.py

@@ -60,7 +64,11 @@ def validate(self, attribute_type=None):
        assert all(isinstance(x, EvaluatorException) for x in self.exceptions)


-class CalculationLayerSchema(AttributeClass):
+class BaseCalculationLayerSchema(AttributeClass):


I added a few base classes... I think this was early on and the code may have actually evolved to never use these, but it seems useful to have a base "this is Evaluator" flag that doesn't contain actual code.

I like base classes

openff/evaluator/properties/density.py

lilyminium · 2025-03-17T21:43:20Z

openff/evaluator/workflow/workflow.py

-                    return return_object
+                    # EquilibrationDataExistsException actually represents
+                    # a "successful" short-circuit
+                    if "EquilibrationDataExistsException" not in str(protocol_results):


This is somewhat hacky but specific enough that it shouldn't be raised in the general case.

openff/evaluator/workflow/workflow.py

openff/evaluator/protocols/storage.py

lilyminium · 2025-03-18T20:29:29Z

openff/evaluator/layers/equilibration.py

+
+            # TODO: should this be hardcoded?
+            # TODO: it is hardcoded in protocols already
+            if key == "component_data":


See todos -- not sure this should be hardcoded, but it is in the protocols already (and for the existing reweighting layer too)

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

lilyminium · 2025-03-18T22:03:23Z

There's a lot of test files here, we should maybe re-visit tarballing files and I should have a look at what should be removed? I was a bit 50/50 on how useful these were as a record of "this is what you can expect during a run" vs whether all these files should be cluttering up the repo. I think most of the files necessary are just the jsons and maybe pdbs and xmls.

mattwthompson

We agreed on the path forward

Merge this approximately as-is (@lilyminium gets to press the button!)
Lily has a follow-up PR soon in the works
Tag a beta release
Run some re-fit(s)
(somewhat ambiguously) go from there

mattwthompson · 2025-03-25T03:01:33Z

Oh, and leaving test files as-is but (later) looking into zipping them

lilyminium · 2025-03-25T03:05:40Z

Thanks for the review and summary @mattwthompson! I'll merge into main and build off it for the next PR.

lilyminium and others added 4 commits January 21, 2025 14:16

add equilibration code

3049504

add some preliminary docs

bd2e208

add some docstrings

53dfa13

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

6132bf6

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

lilyminium mentioned this pull request Jan 23, 2025

Add NoBatch mode #602

Merged

1 task

mattwthompson added this to the 0.4.11 milestone Jan 28, 2025

mattwthompson mentioned this pull request Jan 28, 2025

Release 0.5.0 #599

Closed

lilyminium and others added 17 commits January 29, 2025 10:59

switch greaterthan -> greaterthanorequalto

78d6e5d

add updates for equilibrated properties

4ad2337

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

45e278e

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

fix equilibration_properties

9816186

Add new conditions and tests (#614)

536bcde

* 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>

Migrate tmpdir fixture to new tmp_path (#615)

2c22533

Add AttributeClass initialization (#606)

19163ac

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

f1b3354

* Automatically strip out `openmm.CMMotionRemover` force internally * More consistently strip force

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

fbfc52a

add data

847534f

Merge branch 'main' into add-equilibration-layer

ec3c5c6

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

e5a0958

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

remove additional raise

d9ec993

jaclark5 assigned jaclark5 and unassigned jaclark5 Mar 10, 2025

disallow merging

7b3c5dd

lilyminium commented Mar 18, 2025

View reviewed changes

rm doc stub for now

f30b6ef

lilyminium commented Mar 18, 2025

View reviewed changes

lilyminium and others added 5 commits March 19, 2025 07:35

Update openff/evaluator/workflow/workflow.py

b39c026

Update openff/evaluator/properties/density.py

13d3397

remove unused imports

35c5a68

fix imports

43815b8

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

f0b9322

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

lilyminium marked this pull request as ready for review March 18, 2025 21:44

mattwthompson mentioned this pull request Mar 25, 2025

Re-visit can_merge band-aid #637

Open

mattwthompson approved these changes Mar 25, 2025

View reviewed changes

lilyminium merged commit ac87073 into main Mar 25, 2025
15 of 17 checks passed

lilyminium mentioned this pull request Mar 26, 2025

Add PreequilibratedSimulationLayer #638

Merged

Conversation

lilyminium commented Jan 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

codecov bot commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lilyminium commented Mar 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lilyminium commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattwthompson left a comment

Choose a reason for hiding this comment

Uh oh!

mattwthompson commented Mar 25, 2025

Uh oh!

lilyminium commented Mar 25, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lilyminium commented Jan 21, 2025 •

edited

Loading

codecov bot commented Mar 4, 2025 •

edited

Loading

lilyminium commented Mar 18, 2025 •

edited

Loading