search

openmm / openmm

OpenMM is a toolkit for molecular simulation using high performance GPU code.
1.5k stars 525 forks source link

Final changes for 7.0 #1426

Closed peastman closed 8 years ago

peastman commented 8 years ago

I think we're getting close to being ready to build a release candidate of 7.0. I've been going through the tracker looking for anything that still needs to be resolved, and there's not much left. But I'd like to have everything in one place. So if there's anything you think still needs to be fixed for 7.0, please post it here.

jchodera commented 8 years ago

We can tag issues for 7.0...

jchodera commented 8 years ago

The openmm-dev conda build is still broken on osx:


Fixing linking of @rpath/libOpenMMPME.dylib in /Users/travis/anaconda/envs/_build/lib/plugins/libOpenMMPME.dylib

Fixing linking of @rpath/libOpenMM.dylib in /Users/travis/anaconda/envs/_build/lib/plugins/libOpenMMPME.dylib

Fixing linking of /usr/lib/libSystem.B.dylib in /Users/travis/anaconda/envs/_build/lib/plugins/libOpenMMPME.dylib

Fixing linking of @rpath/./libfftw3f.3.dylib in /Users/travis/anaconda/envs/_build/lib/plugins/libOpenMMPME.dylib

Fixing linking of @rpath/./libfftw3f_threads.3.dylib in /Users/travis/anaconda/envs/_build/lib/plugins/libOpenMMPME.dylib

Fixing linking of /usr/lib/libc++.1.dylib in /Users/travis/anaconda/envs/_build/lib/plugins/libOpenMMPME.dylib

install_name_tool -add_rpath @loader_path/../ /Users/travis/anaconda/envs/_build/lib/plugins/libOpenMMPME.dylib

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/install_name_tool: object: /Users/travis/anaconda/envs/_build/lib/plugins/libOpenMMPME.dylib malformed object (load command 18 cmdsize is zero)

Error: install_name_tool failed with exit status 1

ERROR: File does not exist: /Users/travis/anaconda/conda-bld/osx-64/openmm-dev-0.0.0-py34_0.tar.bz2

Package failed to build (return code 1); will not upload.

We probably need to fix this before we are able to rebuild a new 7.0 release candidate.

peastman commented 8 years ago

When did it break? How did the beta get built?

jchodera commented 8 years ago

When did it break?

Clicking on the travis badge on the conda-dev-recipes page and going to "Build History" gets you the list of travis builds (which are performed nightly) where we can see that it broke in this build.

Unfortunately, I don't know what changes were made that caused this to break. Nobody besides me seemed to want to have these dev packages built by the openmm repo travis, so we're stuck with a case where you having to figure this out by looking through the openmm, omnia-md/conda-dev-recipes, and omnia-md/conda-recipes commit histories.

How did the beta get built?

We built it before someone broke the dev builds, and now we have to unbreak it before we can build the release candidate.

mpharrigan commented 8 years ago

Unfortunately, I don't know what changes were made that caused this to break

You can very easily get the git hashes of the last successful build and the first failing build from the travis logs

commit 9b5b52f2bf2e851efdd0733fed3e83a24d9f16af
Merge: b768ff4 71cb735
Author: peastman <peter.eastman@gmail.com>

Date:   Fri Mar 4 12:54:01 2016 -0800

    Merge pull request #1407 from peastman/coefficients

    Updated default coefficients for extrapolated polarization

edit: cc #1419

peastman commented 8 years ago

Just to keep this up to date, there are currently two issues I know of that need to be resolved for 7.0: the error building the conda package (#1419) and the crashes with AMOEBA (#1427). As soon as those are resolved, I believe we can build the release candidate.

jchodera commented 8 years ago

You can very easily get the git hashes of the last successful build and the first failing build from the travis logs

Thanks!

peastman commented 8 years ago

It looks like the AMOEBA crashes are probably from too large a time step, not from a bug. So as soon as #1442 is merged, I think we'll be ready to build the release candidate. Does that sound OK to everyone?

jchodera commented 8 years ago

Sure.

peastman commented 8 years ago

Ok! I've created the tag 7.0rc1. Let's build the release candidate!

peastman commented 8 years ago

Which recipe should I use to build the Windows conda installers?

jchodera commented 8 years ago

Working on creating new conda recipes...

jchodera commented 8 years ago

Which package/channel should these conda recipes get pushed to? Do we want

peastman commented 8 years ago

Users should not get this by default. Until it becomes the official release, conda install openmm should continue to install 6.3.1. I don't much care what the exact command they do type to get it is though.

jchodera commented 8 years ago

I've created a PR for conda-recipes here. I believe @marscher's changes to conda-build-all mean that we will see these appear with the pre label, requiring the user to invoke

conda install -c omnia/label/pre openmm

to install. The advantage is that we won't have to have the user manually uninstall the existing openmm package.

https://github.com/omnia-md/conda-recipes/pull/470

marscher commented 8 years ago

On 29.03.2016 01:34, John Chodera wrote:

I've created a PR for |conda-recipes| here. I believe @marscher https://github.com/marscher's changes to |conda-build-all| mean that we will see these appear with the |pre| label, requiring the user to invoke

conda install -c omnia/label/pre openmm

to install. The advantage is that we won't have to have the user manually uninstall the existing |openmm| package. Yes - you're right with all you said.

jchodera commented 8 years ago

Thanks for the confirmation, @marscher!

jchodera commented 8 years ago

The openmm 7.0rc1 package is now posted for linux: https://anaconda.org/omnia/openmm/files

Not sure why osx didn't build. Looking into this.

jchodera commented 8 years ago

cc: https://github.com/omnia-md/conda-recipes/issues/472

jchodera commented 8 years ago

Thanks to @marscher, osx packages are up now too: https://anaconda.org/omnia/openmm/files

jchodera commented 8 years ago

I'll build the Jenkins tarballs now.

jchodera commented 8 years ago

OpenMM 7.0rc1 packages:

peastman commented 8 years ago

I've merged #1444 and moved the 7.0rc1. Sorry for making you rebuild!

mpharrigan commented 8 years ago

We really should just issue a new tag 7.0rc2. There's no drawback I can think of, and it has the benefit of disambiguating the previous tag and @jchodera 's packages he already built

jchodera commented 8 years ago

I've merged #1444 and moved the 7.0rc1.

Wait, what? You moved the tag?

jchodera commented 8 years ago

Move it back. We're on 7.0rc2 now. 7.0rc1 conda packages are already out in the wild.

peastman commented 8 years ago

"rc1" means "the first release candidate we release". If we only post one release candidate, and there are two different tags called rc1 and rc2, people won't be able to tell which one goes with the thing we actually released.

This is why I usually prefer to not add tags until after we've already posted the release (so we know what commit it actually went with). But @chodera gets upset when I do that.

peastman commented 8 years ago

Move it back. We're on 7.0rc2 now. 7.0rc1 conda packages are already out in the wild.

What are you talking about? I certainly haven't released anything yet, and you shouldn't have either! Once all the files are ready, I post everything at once.

jchodera commented 8 years ago

Look: You asked me to build the 7.0rc1 conda packages. They were built and uploaded to binstar under the pre tag. They may be on people's machines now, and the only want to make sure that we can update them and check that we have the right version is to build a release with a newer version number. Otherwise, we have to ask anyone who has the old 7.0rc1 to manually remove the package, clear their conda caches, and reinstall, which will just make things confusing. That's why we have version numbers here.

Don't worry, we won't run out of version numbers.

jchodera commented 8 years ago

If you'd like to take over all the conda stuff instead of asking me to do things and then getting confused when I do them, that's fine. It's a huge time sink for me.

peastman commented 8 years ago

Alright, I'm trying very hard not to start screaming.

Here is how releases are going to happen. This applies to all official numbered releases, including ones with "beta" or "release candidate" in the title.

First, we will build and collect all installers, both zip installers and conda packages. Then we will do some minimal testing of them: basically making sure they can install correctly and the testInstallation script runs. Then and only then, we will post them all at once, both to simtk.org and to the appropriate conda channels. Until everything has been built and tested, nothing will be released in any way that end users might get it. That is how releases have always worked, and that is how they will continue to work.

jchodera commented 8 years ago

You and me both.

I suggest you document this process here instead of asking us to read your mind: https://github.com/pandegroup/openmm/wiki/Cutting-an-OpenMM-release

Note that our conda build system doesn't support the scheme you describe. Once we commit the 7.0rc1 recipe to the omnia/conda-recipes repo, it gets built and pushed to anaconda cloud with the pre tag. Someone would need to explicitly request conda install --c omnia/labels/pre openmm to install it at that point---it doesn't get picked up automatically. But if someone has done that here at MSK or elsewhere, I'd need to track them down and have them clean the package out with conda remove --yes openmm ; conda clean -plits --yes in order to get rid of it. That was the situation I was hoping to avoid by having you increment to 7.0rc2 instead.

In your release cycle specification, it sounds like you want the conda packages to be built and tested before they go up to anaconda cloud. You are welcome to do this if you want---you can update the conda recipe locally, build locally with conda build openmm, and test locally if you really don't want to even risk having the release candidate pre-build (???) up on anaconda cloud with the pre label. We don't support this, however, so you'd be on your own for that.

peastman commented 8 years ago

You say that as if this were something new. This is precisely how every release we have ever done has happened. So please understand my frustration. It appears (from my point of view) that you just decided to change our release procedure without consulting anyone or telling anyone about it. We have always tested releases before posting them. This isn't something new.

Someone would need to explicitly request conda install --c omnia/labels/pre openmm to install it at that point

Ok, I don't think that's a big problem, especially since we haven't yet posted those instructions anywhere for end users. In the same way, people can always get the source from github, or the latest openmm-dev build. They don't expect that to be anything official. The important thing is anything we publicly advertise as "a release candidate of OpenMM 7.0".

In your release cycle specification, it sounds like you want the conda packages to be built and tested before they go up to anaconda cloud.

Correct. That's essential for any official release. But again, it isn't that they can't be anywhere in anaconda cloud. They just can't be somewhere that we've advertised to users as containing official releases.

One other important point: building and deploying necessarily need to be decoupled so that, once we accept a release candidate as the official release, we can post it to the main channel without rebuilding it. Because if we rebuild it, that creates more opportunities for something to go wrong, so it's a new release candidate.

mpharrigan commented 8 years ago

One other important point: building and deploying necessarily need to be decoupled so that, once we accept a release candidate as the official release, we can post it to the main channel without rebuilding it. Because if we rebuild it, that creates more opportunities for something to go wrong, so it's a new release candidate.

This is an honorable goal. With the current setup, you'd need to re-build to change the version string

mpharrigan commented 8 years ago

Because if we rebuild it, that creates more opportunities for something to go wrong

See fftw issue on macos x :)

peastman commented 8 years ago

See fftw issue on macos x :)

Exactly!

mpharrigan commented 8 years ago

Moving tags around still seems like a no-no. Maybe it's not ideal, but at this point we need to start calling things rc2 or we will start having to say "the first rc1" and "the second rc1" when talking about builds and issues

peastman commented 8 years ago

Let's put aside tags for the moment then. We should currently be building revision f4339363dcc4c904b2baae05949dedc0676b3f50. Once it's actually posted and we're agreed on what it's called, we can make a tag to record that.

jchodera commented 8 years ago

You say that as if this were something new. This is precisely how every release we have ever done has happened. So please understand my frustration. It appears (from my point of view) that you just decided to change our release procedure without consulting anyone or telling anyone about it. We have always tested releases before posting them. This isn't something new.

If it's not documented in detail, there is no standard procedure. I don't know what you did last time. I don't know what I did yesterday. We are very busy, but I can follow a sufficiently well-documented procedure.

Ok, I don't think that's a big problem, especially since we haven't yet posted those instructions anywhere for end users.

It isn't currently a big problem, but as @mpharrigan points out, moving tags around is VERY BAD. DO NOT DO THIS. It could get us in a situation in which we (1) don't know what code is in the wild and (2) have no way of forcing updates to the latest version.

Correct. That's essential for any official release. But again, it isn't that they can't be anywhere in anaconda cloud. They just can't be somewhere that we've advertised to users as containing official releases.

So you're OK with the release candidate packages going up into the pre label?

One other important point: building and deploying necessarily need to be decoupled so that, once we accept a release candidate as the official release, we can post it to the main channel without rebuilding it. Because if we rebuild it, that creates more opportunities for something to go wrong, so it's a new release candidate.

Again: Are you OK with automatically building the release candidate conda packages onto the conda cloud with the pre label?

Let's put aside tags for the moment then. We should currently be building revision f433936. Once it's actually posted and we're agreed on what it's called, we can make a tag to record that.

Two things here:

Also, PLEASE DOCUMENT THIS PROCEDURE IN DETAIL. Make a checklist with clear roles for each person. I can follow checklists. I can't read your mind or remember what you did last time.

peastman commented 8 years ago

So you're OK with the release candidate packages going up into the pre label?

I don't know what that means. Here is what I am trying to say. Let me know if any of this is not sufficiently precise.

  1. At present, I believe that revision f4339363dcc4c904b2baae05949dedc0676b3f50 may become OpenMM 7.0. I don't yet know for sure. We won't know for sure until after we test it and invite users to try it. If no problems are found, then after about a week we will declare it to be OpenMM 7.0. (That's what "release candidate" means. It's a build that might turn out to be the actual release, but we don't know yet.)
  2. We need to build installers (both zip and conda) for that revision. We should not yet post them anywhere that end users will be looking for them.
  3. Once we have done some minimal testing on them, we need to post them somewhere so that users can access them. We will send out instructions telling them how to do that.
  4. If no problems are found in that build, then we will declare it to be the official 7.0 release and post it to the standard locations. We will not rebuild them, since that would then be a new release candidate.
  5. If, on the other hand, a serious problem is found, we will say, "Ok, that wasn't 7.0 after all." We will then build a new release candidate and start the process over.

Is that sufficiently clear?

jchodera commented 8 years ago

Thanks for being precise here, but please document the procedure as a checklist in the wiki we have for this purpose.

Builds are underway for the zip distributions.

conda packages: As I mentioned earlier, our current scheme does not support the building of conda packages without automatically uploading them to anaconda cloud. Currently, versions tagged as X.YrcZ get uploaded to the pre label on anaconda cloud (like 7.0rc1 was) which is a nice safe place that is not normally distributed to people since you have to specifically say conda install -c omnia/labels/pre openmm to get it---that we do support and I would be happy to do that. Anything else you might want is up to you. I've previously suggested some alternatives that would let you test builds as potential release candidates we could eventually automate, but it would take some time, and you probably don't want to wait a few weeks to sort this out.

How about you put all of that in the wiki as a checklist?. I'm certainly not going to come here to look up this procedure next time.

peastman commented 8 years ago

please document the procedure as a checklist in the wiki we have for this purpose.

I'm on it!

mpharrigan commented 8 years ago

Currently, versions tagged as X.YrcZ get uploaded to the pre label on anaconda cloud

Presumably we'd need a way to upload a version tagged as 7.0 (no rc) to the pre label, and when we're satisfied with it, move it to the "main" label. i.e. what it seems those labels were designed for:

http://docs.anaconda.org/using.html#UsingLabelsInTheDevelopmentCycle

jchodera commented 8 years ago

What about adding a section to the meta.yaml called label where you can choose what label it should be uploaded under?

mpharrigan commented 8 years ago

In fact, that sort of flow would be really useful for all the conda packages. We could have msmbuilder hanging out in pre for a bit and then click a button to unleash it on the world (without building a potentially different binary a thousand times)

mpharrigan commented 8 years ago

What about adding a section to the meta.yaml called label where you can choose what label it should be uploaded under?

Is this supported by conda build?

jchodera commented 8 years ago

Is this supported by conda build?

We would have to add this capability to conda-build-all.

peastman commented 8 years ago

https://github.com/pandegroup/openmm/wiki/Release-Candidates

jchodera commented 8 years ago

Thanks! Looks good.

Some comments:

mpharrigan commented 8 years ago

Is this supported by conda build?

We would have to add this capability to conda-build-all.

Can you add extraneous tags to meta.yaml without conda build complaining?