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

Update imports and tests for OpenMM 7.6 #173

Merged
merged 6 commits into from Sep 8, 2021
Merged

Conversation

jchodera
Copy link
Member

This PR attempts to update all imports and test file paths for OpenMM 7.6, pinning to >=7.6.

Fixes #172

@jchodera jchodera added this to the 0.10.0 milestone Aug 28, 2021
@jchodera
Copy link
Member Author

jchodera commented Aug 28, 2021

@peastman: Tests will not pass and all released versions of openmmtools will be broken until we fix the ParmEd issue: ParmEd/ParmEd#1185

args = (<Topology; 1 chains, 150 residues, 2423 atoms, 2443 bonds>,)
kwargs = {'xyz': [Vec3(x=24.996000000000002, y=49.389, z=27.32), Vec3(x=24.563000000000002, y=50.251000000000005, z=27.02), Vec...00000004, z=26.500000000000004), Vec3(x=23.893, y=48.375, z=27.6), Vec3(x=23.533000000000005, y=48.05, z=26.623), ...]}

    @wraps(fcn)
    def new_fcn(*args, **kwargs):
        if not HAS_OPENMM:
            raise ImportError('Could not find or import OpenMM')
        if not SUPPORTED_VERSION:
>           raise ImportError('You must have at least OpenMM 6.3 installed')
E           ImportError: You must have at least OpenMM 6.3 installed

/usr/share/miniconda/envs/test/lib/python3.9/site-packages/parmed/utils/decorators.py:30: ImportError

@jchodera
Copy link
Member Author

This is blocked until conda-forge/parmed-feedstock#15 is merged and the ParmEd conda-forge packages are released.

@jchodera
Copy link
Member Author

jchodera commented Sep 1, 2021

@peastman : Are new packages not deployed yet? I'm just getting

_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

args = (<Topology; 1 chains, 150 residues, 2423 atoms, 2443 bonds>,)
kwargs = {'xyz': [Vec3(x=24.996000000000002, y=49.389, z=27.32), Vec3(x=24.563000000000002, y=50.251000000000005, z=27.02), Vec...00000004, z=26.500000000000004), Vec3(x=23.893, y=48.375, z=27.6), Vec3(x=23.533000000000005, y=48.05, z=26.623), ...]}

    @wraps(fcn)
    def new_fcn(*args, **kwargs):
        if not HAS_OPENMM:
            raise ImportError('Could not find or import OpenMM')
        if not SUPPORTED_VERSION:
>           raise ImportError('You must have at least OpenMM 6.3 installed')
E           ImportError: You must have at least OpenMM 6.3 installed

@jchodera
Copy link
Member Author

jchodera commented Sep 1, 2021

@peastman : I'll let you sort out the OpenMM 7.6 issues from here. I have no idea what to do.

@peastman
Copy link
Member

peastman commented Sep 3, 2021

I think it's happening because you aren't installing ParmEd directly. Instead you install ambertools and get ParmEd from it.

- ambertools >=18.0 # contains sufficiently recent ParmEd

The constraint on OpenMM version was only added to the parmed package, not the ambertools package. Fixes could include directly listing ParmEd as a dependency or setting an upper bound on OpenMM in test_env.yaml.

@jchodera
Copy link
Member Author

jchodera commented Sep 4, 2021

The constraint on OpenMM version was only added to the parmed package, not the ambertools package. Fixes could include directly listing ParmEd as a dependency or setting an upper bound on OpenMM in test_env.yaml.

@peastman : I don't understand. ambertools depends on parmed:
image

Why wouldn't the constraint solver identify the correct combination of constraints that include the latest OpenMM (7.6.0) and the latest ParmEd compatible with it (3.4.3)? If this isn't happening, we haven't actually fixed the issue. There should be absolutely no need to specify parmed explicitly or an explicit parmed version unless things are still broken for everyone.

@jchodera
Copy link
Member Author

jchodera commented Sep 4, 2021

It is indeed correctly installing parmed 3.4.3 now. We're just running into the bug introduced from merging #156

@jchodera
Copy link
Member Author

jchodera commented Sep 8, 2021

@peastman : Now that GLYCAM support has been merged and ParmEd has been updated, hopefully we can get this cleaned up, merged, and released to provide openmmforcefields support for OpenMM 7.6!

@jchodera jchodera merged commit 73a0707 into master Sep 8, 2021
@jchodera jchodera deleted the fix-tests-for-openmm-7.6 branch September 8, 2021 23:33
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.

Tests failing on OpenMM 7.6
2 participants