-
Notifications
You must be signed in to change notification settings - Fork 506
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
Release thread for OpenMM 8.0 #3610
Comments
Does the deafening silence mean everyone agrees and I should start working on building a beta? |
Yes, let's make a beta, so it is easier to check the NNP functionalities. For now, let's name 7.8. |
@jchodera @giadefa @tmarkland any comments? Which version number do you prefer? |
I think we decided on 8.0 for this one. Proceeding with the beta sounds great to me. |
Apologies for the delay in responding---I've been traveling. The most important functionality to test is to make sure we can make the whole From the "OpenMM 8 status and priorities" document, here are the desiderata I laid out Feb 22:
From this list, I still have to finish up the automated benchmarks PR which I can do quickly so it will be easier for folks to run benchmarks and share them with us. Thoughts on the others? |
Thank you @jchodera and all. I have a draft conda-forge recipe for the ATMMetaForce plugin awaiting review. Yes, the plugin is rather specific to traditional bonded/non-bonded MM force fields and could likely benefit from some level of generalization. Also documentation of the API, unit testing, etc. |
There are several changes that improve it: #3520, #3552, and #3606.
This would be a nice feature for someone to work on. Right now I think it's still at the research stage. Someone needs to experiments with various ways of tuning and see what works well.
If you want to finish it up, that would be great. Since it only affects the benchmark script, I think it's fine if that doesn't happen until after the beta goes out.
It's still very experimental.
This would be nice, and can also be done post-beta.
I'm not convinced that's a good idea. It's certainly something that would need a lot of design and discussion, so not happening in this release. |
@jchodera given the above comments, are you ok with releasing a beta now? It would be nice to have coordinated versions of OpenMM, OpenMM-Torch, and OpenMM-ML so users can test out the full workflow. @raimis do you think the other two packages are also in a suitable state for beta testers to use them, or are there changes we should get in first? @tmarkland has a strong preference for calling this release 8.0. Is that ok with everyone? |
Regarding OpenMM-Torch, it is in a working state after the latest release (https://github.com/openmm/openmm-torch/releases/tag/v0.7). I'm not sure what is the status of OpenMM-ML.
|
@jchodera given the above comments are you ok with moving forward? Everyone else has given their ok. If I don't hear from you by the end of the week, I'll start building the beta. I think OpenMM-ML is in reasonable shape. I just made a PR filling in the README. Once that's merged, I think we can create a 1.0beta release and then build conda packages for it. It would be nice if we could finish up one of the two PRs that adds NNPOps integration first, but since that only affects speed, not features, I don't think it's essential. |
@peastman : What's the status of the beta release? Has it been built / Is it available on conda-forge? If not, when do you expect it to be? |
Not yet. We first need to finish up openmm/openmm-torch#80 so we can build a beta package for OpenMM-Torch, and we need to make changes to OpenMM-ML so it will use the optimized NNPOps kernels. @dominicrufa I think you were going to do the latter? |
@peastman, I can update the |
Excellent, thanks. |
I managed to finish up this PR, which would be great to include in the beta: Otherwise, I think we're ready to go once openmm/openmm-torch#80 is finished? |
I opened a PR to build the beta packages: conda-forge/openmm-feedstock#83. Once it's done, we can build corresponding packages for the plugins. |
Would be great to get #3386 in before we cut the |
It looks like #3386 is going to take a while yet, so I don't want to hold up the beta for it. |
I'm done with that PR, so you can move at your own pace to finish it
however you like. I just think it would be very useful for us to have some
way to capture submitted benchmarks in a machine-readable way.
|
The packages are now online! Next we need to build corresponding packages for the |
I created a PR at conda-forge/openmm-torch-feedstock#23 to build the beta package for OpenMM-Torch. But it looks like we've never set it up to build on any platform other than Linux x86 and Mac x86. I'm not sure exactly what needs to be done to make it build packages for other platforms. And then we need to set up a feedstock to build packages for OpenMM-ML. @mikemhenry and @raimis can you help? This is pushing the limits of my experience building conda packages. |
@peastman what platforms do we want to target? Parity with openmm or a subset? |
As many as possible, but of course it's limited by what PyTorch supports. The conda-forge package includes ARM Mac as well. The official packages on the |
Are there guidelines and suggestions regarding porting plugins? Is there a specific change of the OpenMM API from 7.7 to 8.0 that is likely to break a plugin such as the ATM Meta Force plugin? Thanks. |
There's no API changes. We had to make a number of internal changes related to context handling so it would interoperate correctly with PyTorch. Without those changes, you get crashes in various situations. |
A personal note about HIP if you are still considering it. We have been playing around with the StreamHPC implementation https://github.com/StreamHPC/openmm and https://github.com/StreamHPC/openmm-hip. We were able to build and test it on an AMD card with minor effort. We were also able to port the ATM Meta Force plugin to HIP via the Common platform (great invention btw), again, with very minor effort (https://github.com/Gallicchio-Lab/openmm-atmmetaforce-plugin/tree/hip). We have not done detailed benchmarking, but the performance appears unexpectedly good, probably ~3X better than with OpenCL. We are getting more throughput on an inexpensive and power-moderate AMD RX6750XT card than on a CUDA/V100 accelerator on XSEDE's Expanse. It would be great to have HIP support in a stable OpenMM release soon. |
That's good to know! Thanks for the datapoint. |
Two months later, I think we finally have the problems with building the PyTorch plugin on conda-forge resolved! Which means we can move forward with the beta release. I believe these are the next steps.
|
@mikemhenry @raimis what's the status of the things listed above? The OpenMM and OpenMM-Torch packages have been built, so those are all that's blocking the beta. |
I'll make a PR that details how to create the hosted environment so it will be easier for people to try the beta. Last I checked I can have a status report put together tomorrow morning that triages any blockers. |
New packages are built with the fix from #3912. @mikemhenry we're just waiting for you to fix the problem in the environment file. Then we can announce it. |
Thanks @mikemhenry! I confirmed that If no problems are found, we'll plan to make it an official release in 1-2 weeks. |
I updated the PR+pushed the envs to the cloud |
I'm building an updated release candidate to get the fixes from #3923. They're very minor, localized changes that shouldn't impact any of the downstream packages. They just fix the Python wrappers for two fairly obscure methods. They're mainly important because they impact ForceBalance. |
@peastman I will update the hosted envs, is the only new build for openmm? I'll need to make new builds of the downstream packages so they pull in |
I don't think that's necessary. As noted above, the only change was unrelated to ML. Lee-Ping already confirmed it fixed the problem. For everything else, it's fine if people are testing rc1. |
There have been no further reports of problems. That means it's time to start building the release! I'll create a PR for the OpenMM package, then we can move on to the other packages. Hopefully we can release everything tomorrow. |
Can we hold off until at least Monday? We are still tracking down an issue with running perses / openmmtools with the latest rc. |
What issue? |
I'll ask @ijpulidos and @mikemhenry to summarize their findings, but we're still trying to complete a successful set of local GPU tests of our tools. We have not yet managed to complete this. The other outstanding issues I know of are
There may be others I don't have visibility into right now, but in any case, waiting until Monday seems prudent. @ijpulidos @mikemhenry : Can you make sure to summarize your findings? |
It's Monday and you haven't posted anything further, so I'm assuming it's ok to move forward with the release. If I don't hear from you in the next hour or so, I'll merge the PR to build the packages. |
As far as I can tell the findings are correctly summarized in the last comment from @jchodera . The behavior on how global parameters changed in commit 9d9ffb0 and we have I still have to dig deeper what are the consequences of this behavior in terms of |
That isn't really a change in behavior. It was always wrong and produced undefined results. It's just that it now tells you you're doing something wrong, where before you would silently get different results than you expected. |
@peastman I think we are safe to move forward. |
The packages are building now. Once they're done and I've tested a few of them, I'll move on to openmm-torch and openmm-ml. Once everything's done, we'll be ready to announce it. |
All packages are built. If I install OpenMM and OpenMM-Torch with
it installs the correct packages and all tests pass. If instead I tell it to install OpenMM-ML with
then it incorrectly installs a CPU build of PyTorch:
This causes all tests involving PyTorch to fail with the error
Any idea what causes that? |
I don't get this error, if I do |
The behavior seems to be really erratic depending on the python version, whether you use conda or mamba, whether you install openmm-ml when you create the environment, and probably some other things. Here are some examples.
fails with the error
If I downgrade from python 3.11 to 3.10
then it works and wants to install
If I first create the environment with
then activate it and install openmm-ml with
then it installs a different, older pytorch version:
But if I try to use conda,
then it fails with
|
I figured out how to reproduce the original problem: don't list conda-forge when creating the environment.
That leads it to install
|
I can reproduce the problem using conda:
mamba always works for me, either it gives a working env, or gives an error (torchani is not build for python 3.11). I think once again the issue is conda struggling to deal with pytorch. The newer versions of pytorch seem to need a version of python that comes from the conda-forge channel rather than the main channel. By doing 1. use conda and install python from main when creating the env:
conda installs 2. use mamba and install python from main when creating the env:
mamba installs Solutions
the newest compatible python will be installed from conda-forge
will install the latest version of everything correctly. The order you do things with conda matters, by giving it all requirements at the same time, in a completely clean environment, it can work them out correctly (although slowly). |
Your option 4 doesn't work for me. Installing everything with mamba still produces the error. If someone is creating a completely new environment from scratch, there are ways to make this work. But what if someone wants to install it into an existing environment that already includes other software they want to use along with OpenMM? Is there a way to force it to install the correct pytorch? |
What mamba version do you have? I have version 1.2
On my machine with mamba 1.2 then it does correctly install pytorch in an existing environment, the only time it fails to install things are if you try and use python 3.11 where it complains that it cant install torchani |
Using just conda if I use the flag
|
I posted the announcement on the forum. I also created a PR to update the installation instruction in the OpenMM-ML README. Hopefully the conda issues won't affect too many people. If we start getting reports from people running into them, we may need to investigate the causes and see if there's a way to prevent them. |
I was using mamba 0.10.0. I just upgraded to 0.27.0, which is the newest version available on conda-forge. It now installs a GPU pytorch rather than a CPU one. |
In the latest dev build, the errors on Mac described in #3495 seem to have disappear. We don't know why, just as we don't know why they appeared in the first place. But given that, I suggest we move forward with building a beta of the next release. It contains changes needed for ML.
Are there any final features we want to get in before building the beta? Don't worry about bug fixes and documentation changes. Those can go in after the beta. I'm just asking about new features.
Do we want to call it 7.8 or 8.0?
The text was updated successfully, but these errors were encountered: