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

Permit fits with virtual sites #292

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

mattwthompson
Copy link
Contributor

No description provided.

@j-wags
Copy link
Collaborator

j-wags commented Oct 16, 2023

What's the status of this PR? There's no description or tests. Do fits with vsites run locally using this code? Do the outputs appear to be correct? We can tap @lilyminium or someone else on the OpenFF science team to validate these changes if needed.

@mattwthompson
Copy link
Contributor Author

It runs @lilyminium's MRE without error, I don't know if the results are correct or not

@leeping
Copy link
Owner

leeping commented Oct 16, 2023

I'd like to have a minimal test case if possible, for example, we could fit the energies and forces on a water dimer (we could even use a force field to generate fake QM data.) If we start the optimization from different parameters (p1) than those that were used to generate the fake data (p0), and the optimization converges to p0, then we will know the results are correct.

@mattwthompson
Copy link
Contributor Author

Here's the test case that does not work with 1.9.6: vsites-interchange.tar.gz

What tests currently in the codebase should I model a new set of tests after?

@leeping
Copy link
Owner

leeping commented Oct 16, 2023

I think this would work well: https://github.com/leeping/forcebalance/blob/master/src/tests/test_system.py#L122

The test involves running the optimization in studies/001_water_tutorial, which consists of fitting energies and forces for a number of water clusters (I believe the data set includes a few hundred structures 6-mers and 12-mers). To make the current test more lightweight, I think it should be enough to use a few dozen structures of dimers.

@mattwthompson
Copy link
Contributor Author

That's a pretty heavy test, is there a way to test this behavior with fewer moving parts and files/configuration (akin to a set of unit tests)?

@leeping
Copy link
Owner

leeping commented Oct 18, 2023

You could create an instance of the ForceBalance SMIRNOFF engine from a given .mol2 and force field XML file, calculate the single point energy and force for the structure, and compare it to a reference value.

A similar example for AMBER / GROMACS / TINKER is here:

https://github.com/leeping/forcebalance/blob/master/src/tests/test_engine.py#L116

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.

None yet

3 participants