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

Use lambdas for thread pool tasks #1741

Merged
merged 9 commits into from Feb 16, 2017
Merged

Use lambdas for thread pool tasks #1741

merged 9 commits into from Feb 16, 2017

Conversation

peastman
Copy link
Member

This is the first change that uses a C++11 feature. It includes changes to the CMake script to enable C++11, which means it will no longer work on old compilers that don't support it. We also now require CMake 3.1 or later, since that's when they added the necessary support.

This updates the API for the ThreadPool class to allow tasks to be defined with lambdas. That leads to much cleaner and simpler code.

@jchodera
Copy link
Member

We also now require CMake 3.1 or later, since that's when they added the necessary support.

I've updated the build environment wiki page. Are there other places in the docs where we mention minimum software requirements?

@peastman
Copy link
Member Author

It looks like it's also mentioned in the manual. I'll update that.

@peastman
Copy link
Member Author

AppVeyor is failing, probably because it's still using Visual Studio 2010 (10.0). Anyone know how we tell it to use 2015 (14.0)?

@jchodera
Copy link
Member

See here:

os: Visual Studio 2015

@jchodera
Copy link
Member

BTW, I've signed up for a paid AppVeyor account that has much higher throughput and shorter queue times. I could add OpenMM to my AppVeyor projects if you'd like.

@peastman
Copy link
Member Author

Thanks, that would be great!

Some, but not all, of the travis builds are failing because they're using CMake 2.8. I'm not sure what's causing that. I don't see anywhere in travis.yml that we specify a version.

@jchodera
Copy link
Member

Thanks, that would be great!

I think I may need to be added to the pandegroup members to have the projects show up.

Some, but not all, of the travis builds are failing because they're using CMake 2.8. I'm not sure what's causing that. I don't see anywhere in travis.yml that we specify a version.

Maybe that's the default version of cmake on the ubuntu image? Do you know if the docker (sudo: false) images are failing or the real images (sudo: true)? See .travis.yml.

@peastman
Copy link
Member Author

The "sudo: false" ones are failing.

I'm finding a lot of people who have run into this problem. Every one of them seems to suggest a different solution:

https://genbattle.bitbucket.io/blog/2016/01/17/c++-travis-ci/?
https://jonasw.de/blog/2015/07/22/develop/cplusplus14-on-travis-with-cmake/
http://stackoverflow.com/questions/33196136/travis-ci-update-cmake-using-the-packages-cache

@jchodera
Copy link
Member

Those are the container-based build instances, which are based on Ubuntu 12.04 LTS Server Edition 64 bit. I bet the default cmake is just too old. That last suggestion seems to be the best bet to me.

@swails
Copy link
Contributor

swails commented Feb 16, 2017

Can we not just use one from conda?

@jchodera
Copy link
Member

jchodera commented Feb 16, 2017

The current OpenMM .travis.yml build framework doesn't use miniconda for the build environment. I've been working to streamline and modernize the travis build framework to use miniconda (and also upload the build artifacts) in this PR, but it's not quite ready yet.

@mpharrigan
Copy link

cmake provides a linux binary that you can wget and extract

https://cmake.org/files/v3.7/cmake-3.7.2-Linux-x86_64.tar.gz

@peastman
Copy link
Member Author

os: Visual Studio 2015

That doesn't seem to have worked. The build is still failing, and the log reports

-- The C compiler identification is MSVC 16.0.40219.1
-- The CXX compiler identification is MSVC 16.0.40219.1

which according to https://en.wikipedia.org/wiki/Microsoft_Visual_Studio#History is VS2010.

@peastman
Copy link
Member Author

Can someone explain why travis is reporting an unexpected end of file? The code I added was copied almost exactly from http://stackoverflow.com/questions/33196136/travis-ci-update-cmake-using-the-packages-cache, and I'm not seeing the syntax error.

@mpharrigan
Copy link

I think yaml or travis or both removes extra whitespace within a - so you may need to end each line with ; like the other if's

@peastman
Copy link
Member Author

That was it, thanks!

@jchodera
Copy link
Member

That doesn't seem to have worked. The build is still failing, and the log reports

That's odd. There seem to be examples out there that do exactly this. Do any paths have to be updated?

This old page suggests that it may only be available for paid accounts. I think you have to add me as a member of the pandegroup GitHub org in order to let me activate OpenMM builds on paid AppVeyor, however.

@peastman
Copy link
Member Author

No, it's now included on all images, even the old OS versions.

I think the script may have been doing some things in a really strange way. I'm attempting to clean it up and do things in the standard way, in the hope that will make it work.

@mpharrigan
Copy link

Could cclash be masking the desired cl?

@peastman
Copy link
Member Author

I think I've finally got everything working. Just waiting for the travis OS X build, which hasn't started yet. (It often takes over an hour to start.) Assuming that passes, I think this will be ready to merge.

@peastman peastman merged commit 9963e51 into openmm:master Feb 16, 2017
@peastman peastman deleted the lamda branch February 16, 2017 22:54
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

4 participants