modified makefile for properly supporting python 3 in system install …

[armando-fandango]

…and also modified makefile for properly supporting system install for python 2. Fixes issue # 411 reported on github.

Also made dependency on virtualenv conditional as for sysinstall it is not required to have virtualenv or activate virtualenv.
Installed with make sysinstall_python3, make_sysinstall and Tested with make syscheck.

armando@librenix:~/projects/github/neon$ sudo make syscheck
"Skipping virtualenv check for system install / uninstall"
Running style checks. Number of style errors is...
flake8 --count neon bin/* tests examples benchmark
> /dev/null

Number of missing docstrings is...
pylint --disable=all --enable=missing-docstring -r n
neon | grep "^C" | wc -l
465

Running unit tests...
py.test -m "hasgpu or not hasgpu and not mkl_only" tests/ | tail -1 | cut -f 2,3 -d ' '
Exception ignored in: <bound method NervanaGPU.del of <neon.backends.nervanagpu.NervanaGPU object at 0x7f4b27657e48>>
Traceback (most recent call last):
File "/usr/local/lib/python3.5/dist-packages/nervananeon-2.4.0-py3.5.egg/neon/backends/nervanagpu.py", line 893, in del
self.ctx.detach()
AttributeError: 'NervanaGPU' object has no attribute 'ctx'
PyCUDA WARNING: a clean-up operation failed (dead context maybe?)
cuCtxDetach failed: invalid device context
2 failed,

Two errors still occur in GPU code: However, the intent of this PR is not to fix the GPU portions, but the makefile.

[armando-fandango]

In the above PR, the number of tests failed was reduced to 1, not related to the PR:

`====================================================== FAILURES ======================================================
________________________________________ test_dilated_conv[fargs_tests16-cpu] ________________________________________

backend_default = <neon.backends.nervanacpu.NervanaCPU object at 0x7f0f5fee4198>, fargs_tests = (7, 2, 1)

def test_dilated_conv(backend_default, fargs_tests):

    fsz = fargs_tests[0]
    dil = fargs_tests[1]
    stride = fargs_tests[2]
    be = backend_default

    o1, w1 = run(be, False, fsz, stride, 1, dil)
    o2, w2 = run(be, True, fsz, stride, 1, dil)
    # Verify that the results of faked dilation match those of actual dilation.

  assert allclose_with_out(o1, o2, atol=0, rtol=3e-3)

E assert False
E + where False = allclose_with_out(array([[-6622, 11619, 39178, -17867, 17616, -13393, -17023, -3494, 2915, -2020, -35793, 37675, -18931, 10426, -31960, ... 37881, -69578, -38721, 35254, 8138, 16923, -20529, -20013, -8539, -18421, -27050, -14500, 36606, 187]], dtype=float32), array([[-6622, 11619, 39178, -17867, 17616, -13393, -17023, -3494, 2915, -2020, -35793, 37675, -18931, 10426, -31960, ... 37881, -69578, -38721, 35254, 8138, 16923, -20529, -20013, -8539, -18421, -27050, -14500, 36606, 187]], dtype=float32), atol=0, rtol=0.003)

tests/test_dilated_conv.py:174: AssertionError
------------------------------------------------ Captured stdout call ------------------------------------------------
Network Layers:
Sequential
Convolution Layer 'Convolution_40': 1 x (32x32) inputs, 8 x (28x28) outputs, 0,0 padding, 1,1 stride, 1,1 dilation
Convolution Layer 'Convolution_41': 8 x (28x28) inputs, 8 x (18x18) outputs, 1,1 padding, 1,1 stride, 2,2 dilation
Convolution Layer 'Convolution_42': 8 x (18x18) inputs, 8 x (16x16) outputs, 0,0 padding, 1,1 stride, 1,1 dilation
Linear Layer 'Linear_16': 2048 inputs, 1 outputs
Network Layers:
Sequential
Convolution Layer 'Convolution_10': 1 x (32x32) inputs, 8 x (28x28) outputs, 0,0 padding, 1,1 stride, 1,1 dilation
Convolution Layer 'Convolution_13': 8 x (28x28) inputs, 8 x (18x18) outputs, 1,1 padding, 1,1 stride, 1,1 dilation
Convolution Layer 'Convolution_12': 8 x (18x18) inputs, 8 x (16x16) outputs, 0,0 padding, 1,1 stride, 1,1 dilation
Linear Layer 'Linear_6': 2048 inputs, 1 outputs
------------------------------------------------ Captured stderr call ------------------------------------------------
DISPLAY:neon:abs errors: 1.171875e-02 [0.000000e+00, 5.468750e-02] Abs Thresh = 0.000000e+00
DISPLAY:neon:worst case: 1.533482e+04 1.533477e+04
DISPLAY:neon:rel errors: 5.932112e-07 [0.000000e+00, 4.104107e-03] Rel Thresh = 3.000000e-03
DISPLAY:neon:worst case: 9.538086e+00 9.577393e+00
`

[wei-v-wang]

Thanks for your PR, we will look into bringing your PR in.

[baojun-nervana]

@armando-fandango Some issue for python3 sysinstall is due to the system having default python2.

Have you tried python3 system install under a python3 virtualenv?

[armando-fandango]

@baojun-nervana The sysinstall means I want to install as a shared library at the system level, not in the virtual environment. Hence I did not understand the meaning of your comment : "Have you tried python3 system install under a python3 virtualenv?"

The system install, whether for python 2 or python 3, should not ask for the virtual environment to be installed or to be present.

[baojun-nervana]

@armando-fandango Sorry for the confusion. When I said "python3 virtualenv", I mostly meant to have a system / environment with python3 as default. In that case, paths and links will be ready for python3, e.g. pip will have a softlink to pip3. Virtualenv is an easy way to have that environment ready, and "make sysinstall" will work smoothly.

I agree it is confusing to request virtualenv for sys install.

[armando-fandango]

Yes virtualenv is a nice way to create virtual environments with different versions of python etc., but for production environment, sometimes we install python 3 at system level and python 2 is not available, hence the sysinstall previously did not work for such cases.

[armando-fandango]

Actually from the makefile, it seems like that the intent of original makefile author was to install in virtual environment unless 'sys' prefix was added to install and clean.

I think the makefile should be refactored heavily to allow for different parameters such as --env=virtual or non-virtual, --python=2 or 3, --system etc

[baojun-nervana]

There is a parameter PY = 2 or 3 (line 129) which is mostly used to diff venv for python 2 and 3. We may expand that to differ python and pip command for python 2 and 3.

it will be something like the following:

ifeq ($(PY), 2)
.......
PYTHON_EXE := python
PIP_EXE := pip
else
........
PYTHON_EXE := python3
PIP_EXE := pip3
endif

In the recipes, it can be called as $(PYTHON_EXE) and $(PIP_EXE). The neon_install recipe will be like:

neon_install:
@echo "Installing neon system wide..."
@$(PYTHON_EXE) setup.py install
@echo

This way it will be an integrated recipe for python2 and 3.

To run python3 sysinstall, it will be

make PY=3 sysinstall

@armando-fandango Are you interested in updating the PR to merge the python2 and 3 sys install recipes?

[armando-fandango]

Yes I can update my PR to have the recipes as you mentioned. Sounds good.

…

-- Armando Sent from phone.

On Dec 22, 2017, at 5:18 PM, baojun ***@***.***> wrote: There is a parameter PY = 2 or 3 (line 129) which is mostly used to diff venv for python 2 and 3. We may expand that to differ python and pip command for python 2 and 3. it will be something like the following: ifeq ($(PY), 2) ....... PYTHON_EXE := python PIP_EXE := pip else ........ PYTHON_EXE := python3 PIP_EXE := pip3 endif In the recipes, it can be called as $(PYTHON_EXE) and $(PIP_EXE). The neon_install recipe will be like: neon_install: @echo "Installing neon system wide..." @$(PYTHON_EXE) setup.py install @echo This way it will be an integrated recipe for python2 and 3. To run python3 sysinstall, it will be make PY=3 sysinstall @armando-fandango Are you interested in updating the PR to merge the python2 and 3 sys install recipes? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.