Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix some issues with FF hierarchy, SMIRKS patterns #368

Merged
merged 20 commits into from
Jun 28, 2019
Merged

Fix some issues with FF hierarchy, SMIRKS patterns #368

merged 20 commits into from
Jun 28, 2019

Conversation

davidlmobley
Copy link
Contributor

@davidlmobley davidlmobley commented Jun 25, 2019

Addresses #367 . Changes ultimately to be propagated to smirnoff99frosst repo; this is an interim fix to get the FF ready for use in testing.

Fixes some problems with the test force field (changes later to be propagated to smirnoff99Frosst).

@davidlmobley davidlmobley changed the title [WIP] Fix some issues with FF hierarchy, SMIRKS patterns Fix some issues with FF hierarchy, SMIRKS patterns Jun 27, 2019
@davidlmobley davidlmobley requested a review from j-wags June 27, 2019 18:20
@davidlmobley
Copy link
Contributor Author

davidlmobley commented Jun 27, 2019

@j-wags this is ready to go.

@yudongqiu the force field we're going to want to be fitting is here. (edit by Jeff, to remove ambiguity: The new FF is in test_forcefields/smirnoff99Frosst_experimental.offxml)

@davidlmobley
Copy link
Contributor Author

(That said, @j-wags this is "good to go" in the sense of "this is the force field I want put in here", but it does not deal with the issue of the regression tests. Do you want me to put in some alternate FFs for use in the tests? Or you can do it...)

@j-wags
Copy link
Member

j-wags commented Jun 28, 2019

Thanks, I'll get the tests fixed up :-)

@codecov-io
Copy link

codecov-io commented Jun 28, 2019

Codecov Report

Merging #368 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #368   +/-   ##
=======================================
  Coverage   76.54%   76.54%           
=======================================
  Files          19       19           
  Lines        5585     5585           
=======================================
  Hits         4275     4275           
  Misses       1310     1310

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 1c1abc7...2dc6167. Read the comment docs.

Copy link
Member

@j-wags j-wags left a comment

Choose a reason for hiding this comment

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

Looks good. I've moved the new FF to test_forcefields/smirnoff99Frosst_experimental.offxml. I know we make no guarantees about the contents of test_forcefields/smirnoff99Frosst.offxml, but I figure that we want to minimize changes to it if possible. We can consider merging them in once the new FF gets a release/official version in s99F.

@yudongqiu Note that the new FF is at test_forcefields/smirnoff99Frosst_experimental.offxml.

@j-wags j-wags merged commit 3c34a22 into master Jun 28, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants