Upgrade to Python 3 and newest dlib/opencv versions. #172

Open
bamos opened this Issue Jul 29, 2016 · 14 comments

Projects

None yet

5 participants

@bamos
Collaborator
bamos commented Jul 29, 2016

I only wrote OpenFace with Python 2 by default is because some dependencies at the time only supported Python 2. However that's changed now and I've been wanting to upgrade for months. I'm not sure when I'll get to this so let me know if you're interested in helping out.

@markroxor

Interested. How should we begin?

@emctoo
emctoo commented Aug 22, 2016

I tried, actually more than once. The main obstacle is the dlib. I'm using Arch Linux, I can't compile dlib python interface.

@Dgameman1

@emctoo
As far as I'm aware, dlib is up to date on Arch

https://aur.archlinux.org/packages/dlib/

@qacollective

Hi, has anyone started work on this?

I already have a Dockerfile which builds a Python3.5, dlib, open cv3 and torch environment on an Ubuntu 16.10 distro (or any latest docker base image).

After merrily doing this, I wanted to add OpenFace. Then I found that so far, OpenFace is built on Python 2 and now I'm here.

This may be something I can do. I've not coded seriously in years and am certainly no machine learning expert, but am willing to help where I can. Been getting my hands dirty with Python these last couple of months.

So far, I can import every module except OpenFace.

`root@dockerbox:~# python3.5
Python 3.5.2 (default, Nov 17 2016, 17:05:23)
[GCC 5.4.0 20160609] on linux
Type "help", "copyright", "credits" or "license" for more information.

import cv2
import dlib
import torch
import openface
Traceback (most recent call last):
File "", line 1, in
File "/usr/local/lib/python3.5/dist-packages/openface/init.py", line 6, in
import data
ImportError: No module named 'data'

`

I'll start by spinning up a new Ubuntu container and build OpenFace step by step, using Python3.5 and not installing dependencies until I hit an error (in an effort to update and minimize dependencies). Of course, each step I run, I'll record for comparison against the existing 2 Dockerfiles.

Then, I'll start to run tests and no doubt will have some code changes, hopefully fairly trivial - I've already made changes (on my local) to classifier_webcam.py for Python3.5.

These seem to be some pretty good resources, if I need them:
http://sebastianraschka.com/Articles/2014_python_2_3_key_diff.html
https://docs.python.org/3/howto/pyporting.html

What do people think?

@bamos
Collaborator
bamos commented Jan 23, 2017

Hi @qacollective - great! I think you'll only need minor changes to make OpenFace compatible with Python 3, but please do make sure we don't lose Python 2 compatibility when adding these changes.

-Brandon.

@qacollective
qacollective commented Jan 24, 2017 edited

Hi,

Just as a forewarning: I'm new to git, python, docker and pretty much all the technologies and concepts that OpenFace is built on - so I'm on a steep learning curve here!

With that in mind, if you could take a look at what I've just pushed to https://github.com/qacollective/openface and give me your thoughts - that would be great.

Current status is:

  • Merged 2 dockerfiles into one Dockerfile
  • Dockerfile builds an image successfully in a couple of hours
  • Python 2.7 is still present in the resultant image due to other dependencies from boost and torch
  • With minimal changes to OpenFace code, can now import cv2, dlib and openface in Python 3.5 without errors

Next step(s) seem to be:

  • Run tests (./run-tests.sh)
  • Make changes to make tests pass
  • Run demos

One question I have is: isn't there a UI component to some of the demos? (i.e. classifier_webcam.py) How does this work from inside a docker container? Do I have to do something like this: http://fabiorehm.com/blog/2014/09/11/running-gui-apps-with-docker/ ?

Sorry if I'm not connecting all the dots at the moment ... !

@bamos
Collaborator
bamos commented Jan 24, 2017

Thanks! That looks good at a quick glance. I noticed that you modified some of the dockerfiles in your commit. Can you create a new branch that just has the Python 3 source code changes and send in a PR?

If you want to also help with the web demo, you can test it by forwarding some ports from the Docker container to your host. (Not the GUI forwarding you linked to)

-Brandon.

@qacollective
qacollective commented Jan 24, 2017 edited

Sorry, just to clarify - I understand you'd prefer me to isolate the Dockerfile and Python 3 source code changes in separate branches, but was it the Dockerfiles or the Python 3 code changes you wanted in a PR first?

So far I'm assuming you want the Dockerfile changes now, as the Python 3 changes aren't tested, or perhaps you plan to roll in my Python 3 changes with testing you're already doing?

@bamos
Collaborator
bamos commented Jan 24, 2017

For now, I'd like to keep the Dockerfile unchanged and in Python 2 and I'd just like a PR with your Python file changes.

-Brandon.

@qacollective

Ah, I see. Okay - no worries - I'll revert the Dockerfile changes, keep making amendments for Python 2/3 compatibility and continue test.

@qacollective

Hi, I think the pull requests submitted and referenced in this issue should be a good starting point. I'm happy to make alterations if required. See what you think. Details are in the PRs themselves.

@bamos
Collaborator
bamos commented Feb 3, 2017

Thanks @qacollective! I'm not sure why these changes have caused the tests to fail. I'll take a closer look soon and see what's going on.

-Brandon.

@bamos bamos closed this Feb 3, 2017
@bamos bamos reopened this Feb 3, 2017
@qacollective
qacollective commented Feb 7, 2017 edited

Hi @bamos, note that it may not have been these changes which caused the ./run-tests.sh to fail because when I did this (although I don't know what the Travis CI tests are), I was running the changes against Python3, Dlib19.2, OpenCV3.2 and the latest version of Torch as per here:
qacollective#1
(Reading the PR linked above will help understand the context for this PR)

I suspect that slightly different output may have been produced from the newer foundation technologies. Some of the differences between the nose assertions and actual results are tiny, but the assertions fail nonetheless and I didn't feel comfortable changing the assertions to fit without your okay.

One day soon I might try to pull your OpenFace docker image, login to a container of it, clone my Python 3 PR on top and run the tests in that environment and see what results I get. I'd expect the tests to pass and not have any python 2 backwards compat issues.

Alternatively since you likely already have an OpenFace image sitting on your machine, you'd likely be able to do this quicker.

@qacollective
qacollective commented Feb 22, 2017 edited

Hi @bamos, had a few spare moments today so I pulled your latest python2.7 docker image and ran tests with code from my Python 3 pull request.

I've got all tests passing in python2.7 with one small change to how the pickle files are loaded. You may want to check them out in my repo (master...qacollective:master) ... it appears there are incompatibilities in the way Python pickles are handled between Python 2 and Python 3:
http://stackoverflow.com/questions/11305790/pickle-incompatability-of-numpy-arrays-between-python-2-and-3

My solution is a bit hacky - I check python version and execute one line of code differently depending. I'm not sure if it would also work if you unpickled and re-pickled in python3 as a once-off exercise and if that would be backwards compatible with python2? Would OpenFace users then possibly have to do the same for the models they've trained? Perhaps this hacky solution is okay in that case!

It seems that these passing tests shows that the test failures I was getting in my Python3 environment that I built from my own docker file (see a pull request to myself in my repo) were in fact coming from slight changes to the underlying libraries I'd installed because they were newer than in your docker image.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment