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

A fix for OpenMP problems on OS X #227

Merged
merged 2 commits into from Jun 20, 2019
Merged

Conversation

twesterhout
Copy link
Collaborator

For now, we can borrow FindOpenMP.cmake from PyTorch's repo. They maintain a patched version which seems to work on Travis CI with OS X. If I can lay my hands on a Mac, I'll check whether there's a simpler workaround we can use (see points about AppleClang here https://github.com/pytorch/pytorch/tree/master/cmake/Modules).

@gcarleo @PhilipVinc and other OS X users, please let me know whether this works for you.

@gcarleo
Copy link
Member

gcarleo commented Jun 18, 2019

This works if one install openmp with brew install libomp, but otherwise it fails.
I would strongly suggest to set up openmp as an optional library. If it finds it, we use it. Otherwise nope.

@twesterhout
Copy link
Collaborator Author

This works if one install openmp with brew install libomp, but otherwise it fails.
I would strongly suggest to set up openmp as an optional library. If it finds it, we use it. Otherwise nope.

@gcarleo we have MPI in our dependencies, so one has to do brew install openmpi anyway (or whichever implementation one uses). So what's the problem with doing brew install openmpi libomp instead? OpenMP is way more portable that MPI :)

@twesterhout
Copy link
Collaborator Author

Also, PyTorch doesn't work without OpenMP, so if we're going to rely on it for NNs, we need OpenMP (see pytorch/pytorch#20030).

@gcarleo
Copy link
Member

gcarleo commented Jun 18, 2019

Alright, it's fine... we need to add a note in the web page though, I will do that with the next pip release... In any case, since we are here, would you mind adding numpy to the dependencies to be installed automatically? Thanks!

@gcarleo
Copy link
Member

gcarleo commented Jun 19, 2019

@twesterhout ? Do we add the numpy dependency in setup.py here or in another PR?

@twesterhout
Copy link
Collaborator Author

Do we add the numpy dependency in setup.py here or in another PR?

@gcarleo in another PR I think.

Copy link
Member

@gcarleo gcarleo left a comment

Choose a reason for hiding this comment

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

OK thanks, let's add numpy in another PR then

@gcarleo gcarleo merged commit f0a574f into netket:master Jun 20, 2019
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

2 participants