Fix toolkit tests #227

mikemhenry · 2022-08-23T21:17:44Z

No description provided.

…kit-tests

mattwthompson · 2022-08-23T21:41:54Z

The Python 3.8 failure is cryptic but the 3.10 failures look "correct" to me, and they fail in 10 minutes compared to 30-40, which implies to me OpenEye Toolkits are actually being used

mikemhenry · 2022-08-24T17:21:40Z

Supersedes #225

mattwthompson · 2022-08-29T19:07:40Z

I'm looking to find a more stable and portable way to ship the XML files. I can't for the life of me figure out why the symlinks are going two directories up? It looks like these paths are in the root of the repo.

openmmforcefields/setup.py

Lines 24 to 26 in d4e6a07

    
           os.symlink('../../amber/ffxml/', 'openmmforcefields/ffxml/amber') 
        
           os.symlink('../../charmm/ffxml/', 'openmmforcefields/ffxml/charmm') 
        
           os.symlink('../../amber/gaff', 'openmmforcefields/ffxml/amber/gaff')

mikemhenry · 2022-09-01T05:23:04Z

I do think the root of a lot of issues is not packing data files correctly, we should probably switch to a modern approach
https://setuptools.pypa.io/en/latest/userguide/datafiles.html

mattwthompson · 2022-09-02T16:31:30Z

The thing is, I'm pretty sure the package data is configured properly, or at least uses an approach that's on the better end of current best practices:

openmmforcefields/setup.py

Lines 59 to 73 in d4e6a07

    
           include_package_data=True, 
        
           # Allows `setup.py test` to work correctly with pytest 
        
           setup_requires=[] + pytest_runner, 
        
           entry_points={ 
        
               'console_scripts': [], 
        
               'openmm.forcefielddir' : [ 
        
                   'ffxml = openmmforcefields.utils:get_ffxml_path', 
        
                   ], 
        
               }, 
        
           package_data={ 
        
               'openmmforcefields': ['ffxml/amber/*.xml', 'ffxml/amber/gaff/*.{xml,dat}', 'ffxml/charmm/*.xml'] 
        
               },

Exactly why symlinks should be used is not clear to me; they're not portable and it saves no space on disk. I've attempted to remove them altogether with #229

Add boilerplate `.gitignore` file Do not pull from non-standard labels Install instead of update Try to figure out why if: {{ false }} still triggers Syntax? Tinker Bring in (full) mamba Bash thing everywhere Bring down old toolkit Do not present un-loadable ff14SB port as available More pins

https://stackoverflow.com/a/60579142

mattwthompson · 2022-09-15T15:17:42Z

After excessive and inefficient iterations on my part, I think this is ready for review. It includes at least the following changes:

Fixing a missed case from Update for upcoming OpenFF Toolkit API breaks #191
In general, gets tests passing with both the old and new APIs of the OpenFF Toolkit
Updates some tests and iterator names to avoid referring to particles in OpenFF Molecule objects, which do not have the concept of particles
Switches CI to use micromamba
Fixing how package data is handled Fix/remove sym links #230
Adds testing on Python 3.10 (macOS only) Turn off macos + Python 3.10 tests #234
Brings code coverage reports online
- https://app.codecov.io/github/openmm/openmmforcefields/pull/227
- The reports' diffs might not be useful until the default branch is updated in CodeCov's settings
Updates the .gitignore file to make my screen less red
Updates some Python 2 cruft and sorts imports
Removes a stray simtk import
Adds openmmforcefields/tests/__init__.py in order for tests to be recognized as a module

There's more I'd like to do, but this is already borderline unreviewable due to my lack of restraint. I can't do much more than point to that the automation has been improved and tests are passing.

mattwthompson · 2022-09-15T15:46:30Z

I can't request a review via the UI since you opened it, so a ping will have to do @mikemhenry

mattwthompson

I approve (I think?)

mikemhenry · 2022-09-15T18:30:40Z

I'll review this PR after lunch to just double check it, but it should be good to merge in, like we don't have others we want to merge in first, right? @mattwthompson

mattwthompson · 2022-09-15T18:33:18Z

Nope, the other PRs have either been merged into this one or were experiments that didn't work out. It's just this one (for now)!

mikemhenry · 2022-09-15T20:11:16Z

.github/workflows/ci.yaml

-    - name: CodeCov
-      if: ${{ github.repository == 'openmm/openmmforcefields'
-              && github.event != 'schedule' }}
+    - name: Upload coverage report to CodeCov


I will add a quick guard to not upload coverage reports when it is scheduled CI

.gitignore

.github/workflows/ci.yaml

.gitignore

mikemhenry · 2022-09-15T20:17:20Z

@mattwthompson I will take over from here! I just have a few things I want to tweak before I get it merged in, again, thanks for all your help!

mattwthompson · 2022-09-15T20:32:41Z

Great!!! Thank you!

mattwthompson and others added 17 commits August 9, 2022 04:26

Ensure 0.11.0 RC version of toolkit is installed

bdf5ebb

Possible fix to finish #191

0478946

Try to ensure bash args thing is passed through

78c69c9

Naive unit refactor

2401e69

Work through tests

aaa611c

Update CI matrix

6d687a6

Use mamba for development

53c83c4

Debug, add boilerplate .gitignore

c523e36

Fix syntax

0833632

make sure the package didn't get installed before we install it

dfeaf59

Merge branch 'main' into fix-toolkit-tests

8c7424f

Merge remote-tracking branch 'origin/fix-toolkit-tests' into fix-tool…

3495656

…kit-tests

Do not install Espaloma

dbb7fa1

Fix some unit registries in tests

f06a347

install espaloma, but remove openmmforcefields

b3a8af9

Merge branch 'main' into fix-toolkit-tests

bfad5e8

add back in espaloma

eaf7806

switch to micromamba

adb2707

mattwthompson added 4 commits August 30, 2022 09:29

Explicitly move ffxml sources into entry point

6af78a9

Remove symlinking

bc9c21c

Add boilerplate .gitignore

5a27072

Update package installation in CI

99aa521

mattwthompson added 3 commits September 6, 2022 13:53

Fix finding Amber files in test

edade4e

Soft import whatever units module is used by the toolkit

e5e0f19

mattwthompson added 11 commits September 13, 2022 22:42

Fiddle with path

ed42f7b

Just try to get stuff passing

432985e

Try uploading again

a3b3cd0

Switch back to codecov

fe720f2

Simply coverage reporting

563fcef

Allow 0 coverage

663fc10

Simplify coverage arguments

5e58a8e

Add __init__.py in /tests/ submodule

97d9ee8

https://stackoverflow.com/a/60579142

Add __init__.py in /tests/ submodule

9538308

https://stackoverflow.com/a/60579142

Simplify coverage arguments

1b666b5

Add back durations argument

59d99b1

Merge branch '227-codecov' into fix-toolkit-tests

26b6468

mikemhenry requested a review from mattwthompson September 15, 2022 18:14

mattwthompson approved these changes Sep 15, 2022

View reviewed changes

mikemhenry commented Sep 15, 2022

View reviewed changes

mikemhenry added 2 commits September 15, 2022 13:18

Apply suggestions from code review

0df6c4c

only upload report if it is not scheduled

39af8e6

mikemhenry enabled auto-merge (squash) September 15, 2022 20:25

mikemhenry merged commit 35496e5 into main Sep 15, 2022

mikemhenry deleted the fix-toolkit-tests branch September 15, 2022 20:59

Fix toolkit tests #227

Fix toolkit tests #227

Uh oh!

Conversation

mikemhenry commented Aug 23, 2022

Uh oh!

mattwthompson commented Aug 23, 2022

Uh oh!

mikemhenry commented Aug 24, 2022

Uh oh!

mattwthompson commented Aug 29, 2022

Uh oh!

mikemhenry commented Sep 1, 2022

Uh oh!

mattwthompson commented Sep 2, 2022

Uh oh!

mattwthompson commented Sep 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattwthompson commented Sep 15, 2022

Uh oh!

mattwthompson left a comment

Choose a reason for hiding this comment

Uh oh!

mikemhenry commented Sep 15, 2022

Uh oh!

mattwthompson commented Sep 15, 2022

Uh oh!

mikemhenry Sep 15, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mikemhenry commented Sep 15, 2022

Uh oh!

mattwthompson commented Sep 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mattwthompson commented Sep 15, 2022 •

edited

Loading