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

bump pyemma 2.3 #662

Closed
wants to merge 6 commits into from
Closed

bump pyemma 2.3 #662

wants to merge 6 commits into from

Conversation

marscher
Copy link
Contributor

@marscher marscher commented Jan 6, 2017

No description provided.

@marscher
Copy link
Contributor Author

marscher commented Jan 6, 2017

@jchodera What has changed with the holy build box image?

The build fails because mdtraj can not find the libcxx version it depends on:

 from mdtraj.formats.dtr import DTRTrajectoryFile

ImportError: /usr/lib64/libstdc++.so.6: version `GLIBCXX_3.4.11' not found (required by /anaconda/conda-bld/pyemma_1483724397290/_b_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placeho/lib/python3.4/site-packages/mdtraj/formats/dtr.cpython-34m.so)

@marscher
Copy link
Contributor Author

marscher commented Jan 6, 2017

the windows build is broken for an unknown error:
conda/conda#4072

@rmcgibbo
Copy link
Contributor

rmcgibbo commented Jan 6, 2017

@marscher Your issue is coming from #634. See also #645

@marscher
Copy link
Contributor Author

marscher commented Jan 6, 2017

Thanks for the hint. So we have to build on conda-forge and then upload to omnia, like mdtraj did before?

@marscher
Copy link
Contributor Author

marscher commented Jan 6, 2017

or just disable the import test... great.

@rmcgibbo
Copy link
Contributor

rmcgibbo commented Jan 6, 2017

IIRC the intention is to move as much as possible over to conda-forge and away from omnia.

@marscher
Copy link
Contributor Author

marscher commented Jan 6, 2017

Yes. We can also directly stop to support it. Our packages are already there.

@rmcgibbo
Copy link
Contributor

rmcgibbo commented Jan 6, 2017

You might consider just removing the recipe from here then, as @mpharrigan did for MDTraj. https://github.com/omnia-md/conda-recipes/tree/master/mdtraj

@mpharrigan
Copy link
Contributor

@marscher , if your packages are on conda-forge, you can copy the binaries over to omnia without rebuilding them https://docs.continuum.io/anaconda-cloud/cli#copy

A lot of documentation has conda install -c omnia [packagename], so we should still support the omnia channel even if it's just a copy of the conda-forge channel

@marscher
Copy link
Contributor Author

marscher commented Jan 6, 2017

with the neat side effect of breaking the assumptions of the compatibility of this channel. Why it was not considered to just copy the conda-forge recipe here and let the build system do the rest? Or was it broken (again)?

The only downside of conda-forge is its slowness (eg. the Appveyor queue takes ~3 days per commit)

@mpharrigan
Copy link
Contributor

Luckily, the only compatibility issues that have been reported have been with the omnia build box and not with any users. Yes, we are increasing the system requirements from centos 5 (eol in < 2 months) to centos 6. We're easing over to conda-forge like this:

  • build everything on omnia
  • build recipes on omnia and conda-forge
  • build recipe on conda-forge, support omnia channel for backwards compatibility <---- mdtraj is here
  • build everything on conda-forge

@rmcgibbo
Copy link
Contributor

rmcgibbo commented Jan 6, 2017

@marscher: I think your comment "Or was it broken (again)?" reflects the core reason for moving to conda-forge, despite the trickiness in the interim period that we're seeing. The maintenance burden of keeping the omnia build system humming is pretty high (thus it being frequently broken), and will be reduced substantially with conda-forge because of the wider community adoption.

@jchodera
Copy link
Member

jchodera commented Jan 7, 2017

IIRC the intention is to move as much as possible over to conda-forge and away from omnia.

I think everyone wants to to do this, but the dependencies limit us to a plan that is roughly:

  • release OpenMM 7.1
  • create conda-forge openmm experimental build to see if we can add CUDA and OpenCL support
  • bump omnia to a build image based on the CentOS 6 holy-build-box once this PR is complete, which would harmonize conda-forge and omnia CentOS versions and make it much easier to move individual packages one at a time

The mdtraj experiment with migrating to conda-forge was just too early, and we'll just need to restore the recipe in Omnia in order to avoid these problems from happening over and over again until we're doing CentOS 6 based builds for omnia as well.

@mpharrigan and @rmcgibbo : I'd like to restore the MDTraj recipe into omnia for now. Can you help, or should I take a stab at it?

@mpharrigan
Copy link
Contributor

Hi John,

Not all packages depend on OpenMM. For example, mdtraj and pyemma :)

bump omnia to a build image based on the CentOS 6 holy-build-box once this PR is complete

I think it would be a mistake to (1) wait for this (appears to be stalled) and (2) reconfigure our whole build system instead of using the system conda-forge has set up

in order to avoid these problems from happening over and over again

I think removing the automated import test is a small price to pay for forward progress. In this particular case, @marscher already has conda-forge binaries that have been import tested. We can copy those over to the omnia label for backwards compatibility with conda install -c omnia pyemma

@jchodera
Copy link
Member

jchodera commented Jan 8, 2017

For example, mdtraj and pyemma :)

No, but many do, and many also depend on mdtraj and pyemma, so having mdtraj break things (like this) is problematic at this point.

I think it would be a mistake to (1) wait for this (appears to be stalled) and (2) reconfigure our whole build system instead of using the system conda-forge has set up

Moving omnia to a CentOS 6 based build system is necessary to ensure that we can build packages on conda-forge and copy them to the omnia channel without breaking things. Because you've done that too early, things are starting to break.

Updating to a CentOS 6-based holy-build-box is simply the easiest way (in terms of labor) for us to migrate omnia to CentOS 6. That's not the only way, of course. If we want to create a version of the omnia-build-box docker images based on another CentOS 6 image, that works too, but someone would need to invest the time in it.

I think removing the automated import test is a small price to pay for forward progress. In this particular case, @marscher already has conda-forge binaries that have been import tested. We can copy those over to the omnia label for backwards compatibility with conda install -c omnia pyemma

We run the risk of building packages that don't work and causing lots of pain as a result, since we have no way of strictly enforcing everyone testing their packages separately. And once pyemma moves over, it's going to break something else that uses pyemma, and so on, and so on. Eventually, most packages will be forced to migrate except for those dependent on openmm, which are stuck until we get that built on conda-forge, which won't be trivial.

There's a logical way to do this migration, which is as I outlined. But the "defect early, don't care if we screw others" model is not really helping.

@jchodera
Copy link
Member

jchodera commented Jan 8, 2017

That said, @mpharrigan, if you'd like to put in the effort to create a new set of omnia-build-box docker images (maybe create a different repo, omnia-build-box-6?) to replace the current ones, we could migrate the build system to CentOS 6 much more quickly and then projects that don't depend on OpenMM can migrate over to conda-forge at their own pace. I spent much of my Christmas break fixing the build system for OpenMM 7.1, so have to focus on other things at the moment, but there's a start at basing this on the conda-forge linux-anvil here if you want to give it a try!

@marscher marscher closed this Jan 8, 2017
@marscher
Copy link
Contributor Author

marscher commented Jan 8, 2017 via email

@jchodera jchodera reopened this Jan 8, 2017
@jchodera
Copy link
Member

jchodera commented Jan 8, 2017

There are a few simple solutions to this.

@marscher : Do you really use mdtraj >= 1.8 features? If not, we can simply delete the 1.7 package from the omnia channel.

Alternatively, I'm preparing a PR to temporarily restore mdtraj 1.8 builds to omnia until someone is willing to put in the effort to update our docker build containers to CentOS 6.

@jchodera
Copy link
Member

jchodera commented Jan 8, 2017

@marscher : This PR is working now. Can you check it when you have a chance?

You'll need to rebase or merge from master since I had already separately fixed progress_reporter in #664 after you mentioned the issue.

@marscher marscher closed this Feb 6, 2017
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.

4 participants