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

0.10.x merge into main #1235

Merged
merged 16 commits into from Sep 14, 2023
Merged

0.10.x merge into main #1235

merged 16 commits into from Sep 14, 2023

Conversation

mikemhenry
Copy link
Contributor

Description

Motivation and context

Resolves #???

How has this been tested?

Change log


mikemhenry and others added 14 commits July 12, 2023 13:29
* run CI on new 0.10.x branch

* Update .github/workflows/CI.yaml

Co-authored-by: Iván Pulido <2949729+ijpulidos@users.noreply.github.com>

---------

Co-authored-by: Iván Pulido <2949729+ijpulidos@users.noreply.github.com>
* update comments

* bump ci

---------

Co-authored-by: Iván Pulido <2949729+ijpulidos@users.noreply.github.com>
Co-authored-by: Mike Henry <11765982+mikemhenry@users.noreply.github.com>
* use python executable from env

* run tests using the shell

* parmed 4 seems to be giving us some issues

* actually I think we want the newer parmed

* pin pymbar for now

* ooof, parmed != pymbar

* add some more debugging to figure out how we are getting old parmed

* Update devtools/conda-envs/test_env.yaml

* see if shell=True fixes it

* removed parmed by mistake
* switch to using pymbar 3 & 4 support from openmmtools

* fix typo on import

* fix yaml

* switch to using pymbar 4

* missed a pymbar import

* missed another one

* bump ci

* missed a import

* go back to how it was

* Update devtools/conda-envs/test_env.yaml

---------

Co-authored-by: Iván Pulido <2949729+ijpulidos@users.noreply.github.com>
…ipeline (#1210)

* Initial and final topologies serialized per phase.

* Using properties instead of private attrs.

* Fix test

* Remove uneeded code/attributes

* bump ci

* Store phase topologies separately

* Fix tests. vacuum topologies expected.

* Better docstring.

---------

Co-authored-by: Mike Henry <11765982+mikemhenry@users.noreply.github.com>
* Added note about example for adding oe license

* make docker file much simpler

* build images in CI

* forgot to add conda-forge

* fix permissions on a step

* get oe_license file mounted in docker container

* mount path must be absolute

* setup singulairty to test + fix testing on docker image

* add some testing deps to the image

* add -v for tests, fix envar

* tests are failing, want to test rest of pipeline

* use latest openmm version

* hardcode perses version for now

* bump ci

* make sure we can make openeye dir

* support a dev build as well

* don't hardcode value

* fix name clash

* forgot to add conda-forge

* bump ci

* test docker image and fix missing deps

* install ps

* also push latest tag

* don't build on tag since the conda-forge package won't exist yet

* don't test the examples

* Remove docker deb build, we can do these ourselves

* better document container use

* build a latest version for apptainer

* build with 11.2 to make things more compatable

* skip docker test to see if other bits build okay

* see if I can get the step to fail if there is an error

* skip docker tests to make sure apptainer builds okay

* Add mpiplus and mpi4py to docker image

* give correct path to oe license

* add mpi stuff to docker

* clean up diskspace before build

* skip tests for singularity now that the only failures come from a package bug
* Improving examples dir structure and readme

* Adding Tyk2 CLI example

* removing new-cli/ripk2 example (deprecated)

* fix typo for link

* Clarify tyk2 cli example docs
* CI miscellaneous fixes (#1217)

* CI minor fixes. Allow codecov to fail.

* bump ci

---------

Co-authored-by: Mike Henry <11765982+mikemhenry@users.noreply.github.com>

* Changing offline freq default to checkpoint interval

* Fixing input yaml for example

* commenting offline-freq param

---------

Co-authored-by: Mike Henry <11765982+mikemhenry@users.noreply.github.com>
* peptide mutation MPI example added

* better documentation of motivation
* set cutoff distance in sterics_custom_nonbonded_force

* matching cutoff for custom forces. Improving logic.

* test for HTF nonbonded cutoff

---------

Co-authored-by: Iván Pulido <2949729+ijpulidos@users.noreply.github.com>
* CI miscellaneous fixes (#1217)

* CI minor fixes. Allow codecov to fail.

* bump ci

---------

Co-authored-by: Mike Henry <11765982+mikemhenry@users.noreply.github.com>

* added dels to contexts

* Update perses/app/setup_relative_calculation.py

---------

Co-authored-by: Iván Pulido <2949729+ijpulidos@users.noreply.github.com>
* fix spectator support. Enabling test.

* Test to run on GPU CI.
@codecov
Copy link

codecov bot commented Sep 12, 2023

Codecov Report

Merging #1235 (5b452b4) into main (ae55801) will increase coverage by 0.36%.
The diff coverage is 72.80%.

Additional details and impacted files

@mikemhenry
Copy link
Contributor Author

Getting some interesting errors from pymbar
E AttributeError: 'MBAR' object has no attribute 'getFreeEnergyDifferences'

https://github.com/choderalab/perses/actions/runs/6160980084/job/16719308140#step:9:818

I had thought we fixed that in openmmtools, it is pulling in the latest version

@ijpulidos
Copy link
Contributor

@mikemhenry it should be fixed, it should catch the attribute error and handle that in https://github.com/choderalab/openmmtools/blob/main/openmmtools/multistate/multistateanalyzer.py#L1924

@mikemhenry
Copy link
Contributor Author

@ijpulidos
Copy link
Contributor

It seems like we do need to use pymbar 3.1.1 for GPU tests to pass

@mikemhenry
Copy link
Contributor Author

We can keep that in mind for the conda-forge recipe, is this good now to merge in @ijpulidos ?

Copy link
Contributor

@ijpulidos ijpulidos left a comment

Choose a reason for hiding this comment

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

Just a comment, but not a blocker. This looks good to me!

- pdbfixer
- pip
- progressbar2
- pymbar <4
- pymbar
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to pin the pymbar version here as well or only in the conda-forge recipe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I rather do it in the conda-forge recipe since if we don't have a pin in the yaml we use for tests, it is really easy then in CI to make a matrix testing different versions or like the way we do it now, since there is no pin, cpu tests get pymbar 4, but in our GPU CI yaml, we use the extra build args to then pin pymbar<4. If we put it in the env yaml, we would get an error because micomamba doesn't know what to do when the yaml says >3 but the cli command says <4

@mikemhenry mikemhenry added this pull request to the merge queue Sep 14, 2023
Merged via the queue into main with commit 2329fed Sep 14, 2023
15 checks passed
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