dbrgn / heroku-buildpack-python-sklearn

Python buildpack with Numpy 1.7.0, SciPy 0.11.0 and a current version of Scikit-learn.
MIT License
31 stars 28 forks source link

Add the atlas folder to LD_LIBRARY_PATH #1

Closed ogrisel closed 11 years ago

ogrisel commented 11 years ago

To make the build pack work and able to unpickle sklearn models built offline I add to:

$ heroku config:set BUILDPACK_URL=https://github.com/dbrgn/heroku-buildpack-python-sklearn/
$ heroku config:add LD_LIBRARY_PATH=/app/.heroku/vendor/lib/atlas-base/atlas:/app/.heroku/vendor/lib/atlas-base:/app/.heroku/vendor/lib/

otherwise I get a:

ImportError: liblapack.so.3gf: cannot open shared object file: No such file or directory

when the application loads scipy.

Presumably setting LD_LIBRARY_PATH directly in the build back will remove that manual configuration step.

dbrgn commented 11 years ago

Hm, will that work without having the user-env-compile lab turned on?

ogrisel commented 11 years ago

It just tried to use my master branch where this patch-1 branch has been merged on a newly created heroku app and it seems to work. I don't know where the user-env-compile is set.

dbrgn commented 11 years ago

Alright, I'll test it next week and merge it afterwards :)

ghost commented 11 years ago

Has this been fixed/merged? I was able to push successfully after the issue #2 fix but now I'm getting this same import error ogrisel describes above when I try to migrate my heroku database.

dbrgn commented 11 years ago

@ogrisel Sorry, I was in my vacations (camping in the mountains -> no internet) when this PR was posted and totally forgot about it afterwards...

Adding the atlas folder to LIBRARY_PATH seems to make sense, but I don't quite understand what this pull request does about it. First you're adding the path, but then you're removing it again in the second commit. If you take a look at the PR diff, you can see that you're not really changing anything (besides some refactoring steps concerning references to environment variables).

In the second request, you're writing (LD_)LIBRARY_PATH should be updated by sourcing the numpy/scipy step, but you're not sourcing anything. Did you forget to push some commits?

@buildingspeak Did you try setting the Heroku config variables as described in this pull request description?

ghost commented 11 years ago

Yes, adding the config variable LD_LIBRARY_PATH as ogrisel posted worked for me on removing the liblapack error. I didn't use his actual fork; I just set the config variable in Heroku, pushed again, and it worked.

However, could also be the sequential installing (see my post on issue #2) that really did it; I don't know for sure and I haven't tried to clear that config variable and push again. I'm just happy to have it working! If it would help, I can clear the variable and see if I can still push.

ogrisel commented 11 years ago

but you're not sourcing anything. Did you forget to push some commits?

No, the numpy install script is already sourced in the current master of the buildpack.

ogrisel commented 11 years ago

Here it is:

https://github.com/dbrgn/heroku-buildpack-python-sklearn/blob/master/bin/compile#L158

dbrgn commented 11 years ago

Ah, now I get it. It is sourced in between the two changes.

Would you mind squashing your commits into a single one? With two commits, the easiest way to do that would be:

git reset --soft HEAD^
git commit --amend

Then (after checking the result with git log) do a force push to your Github branch.

ogrisel commented 11 years ago

Done!

dbrgn commented 11 years ago

Great, thank you!

@buildingspeak could you test whether it works now without having the Heroku config variables defined?

ghost commented 11 years ago

I unset the LD_LIBRARY_PATH config var and was able to push and then to migrate the db on heroku, so it seems to be working fine without the config var defined.

dbrgn commented 11 years ago

Very good, thanks for testing it, @buildingspeak.