Skip to content
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

Add Equilibration and PreequilibratedSimulation phases #594

Closed
wants to merge 60 commits into from

Conversation

lilyminium
Copy link
Contributor

@lilyminium lilyminium commented Jan 6, 2025

Description

Addresses #593

This PR adds EquilibrationLayer and PreequilibratedSimulationLayers to Evaluator as one solution to #593. The envisioned use is:

  • running properties through an EquilibrationLayer to equilibrate within a particular potential energy tolerance
  • Use the equilibrated boxes as input to a PreequilibratedSimulationLayer, which runs through all steps of a regular SimulationLayer except box-packing.

This PR branches off #589.

Not currently addressed (perhaps in another PR?):

  • relative_tolerance is unsupported for EquilibrationLayer due to its application as relative_tolerance * measured_property.uncertainty, which requires the units to match the original property. Instead, the absolute_tolerance can be specified in kJ/mol for potential energy.
  • relative_tolerance may not result in successful properties (issue Relative uncertainty does not result in successful properties unless relative_uncertainty > 1.0 #592)
  • properties that aren't Densities or EstimatibleExcessProperties (and only EnthalpyOfMixing has actually been tested) are still unsupported, although it should be easy to extend
  • the equilibrated boxes must be present in the storage backend at the moment instead of being in a separate directory. They're not very human-readable either.
  • PreequilibratedSimulationLayer requires pre-equilibrated boxes to have been computed with a EquilibrationLayer beforehand (the storage query should check for this in a hardcoded way), to avoid contamination from other stored data.
  • have not yet tested re-use of equilibrated boxes across range of force fields
  • have not yet figured out what should happen if multiple boxes are present per system in the stored data. Right now there's a hard-coded check for only finding one box. This should get replaced with something that sorts results somehow and picks one.
  • It would be ideal to have the EquilibrationLayer also query the storage backend and exit early if a result already exists.

TODO:

  • a full test run with properties of concern, and benchmark GPU performance on k8s.
  • update the tutorial notebook once it has finished executing slowly on my laptop

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@lilyminium
Copy link
Contributor Author

Closing for now as this is still very early stages.

@lilyminium lilyminium closed this Jan 8, 2025
@mattwthompson
Copy link
Member

This was looking good, but I'll let you decide where and when you want this to go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants