Conda fixes #5290

mingwandroid · 2016-11-25T23:04:36Z

Because I pushed a version of this branch which was the same as master, Github seems to have closed my old PR against it. Reopening here instead, shame we lost the conversation thread.

I made some test releases for all OSes that Conda supports on https://anaconda.org/rdonnelly from the latest code as of about 11am UTC. This PR has been rebased to the latest master branch.

If you have time to check them I'd appreciate it.

The conda-recipes are at:
https://github.com/mingwandroid/conda-recipes/tree/master/libgpuarray (this one has two patches, I can make a PR for them if you wish)
https://github.com/mingwandroid/conda-recipes/tree/master/python/pygpu (again, two patches).
https://github.com/mingwandroid/conda-recipes/tree/master/python/theano

mingwandroid · 2016-11-25T23:25:22Z

I should say, I plan to rebuild the theano packages when rc1 is ready so people can test them.

nouiz

I have one small change before merging and one comments/questions in case you know better then me.

Thanks

nouiz · 2016-11-28T15:34:27Z

theano/configdefaults.py

+        # Add sys.prefix/lib to the runtime search path. On
+        # non-system installations of Python that use the
+        # system linker, this is generally neccesary.
+        if sys.platform in ("linux", "darwin"):


@abergeron I think this is good and would help fix Mac problem. Just to make you know about this change to be merged soon.

If it does help on mac, that's great. But from what I observed I doubt it. We should test that.

nouiz · 2016-11-28T15:37:53Z

theano/configdefaults.py

@@ -430,7 +430,7 @@ def f(suffix=suffix):
        except OSError:
            rc = 1
        if rc != 0:
-            _logger.warning("conda g++ not available, use `conda install m2w64-toolchain`")
+            _logger.warning("g++ not available, if using conda: `conda install m2w64-toolchain`")


I'm not sure about this warning. I think it will be triggered too often. People not using conda could need the code after this and won't have mkl. I see 2 possibles fix:

Do the check for conda and keep the warning here. I prefer this option, I think it is fine to have only one place with a check for conda. This will ensure that conda user use mkl.

Just record here that mkl isn't available. Then if we can't find a blas in this function, raise this warning. So mostly, postpone this warning to the end of this method in case we don't find a blas.

OK, I will change it to look for "Continuum", or anything containing "conda" as that will get both "conda-forge" and "Anaconda".

nouiz · 2016-11-28T15:41:14Z

theano/__init__.py

@@ -31,10 +31,6 @@

 import sys

-if sys.platform == 'win32' and sys.version_info[0:2] == (3, 5):


Here is an error report with WinPython 3.5 on windows:

#4664 (comment)

I don't have objection to removing the check, but I fear that there is still some python 3.5 on windows that have problems. conda and the official python are fine from what you tell. Do you have an idea why winpython have a problem?

People with python 3.5 not support will still get an error, just an error very hard to understand. If you have an idea how we could detect here if we support the current Python 3.5 on Windows, it would be great.

Otherwise, I think we will keep this and probably add a note in the installation of windows that it is not all python 3.5 that are supported.

notoraptor · 2016-11-28T19:23:34Z

Hello ! Just to say that I have just tested this PR with the test theano.misc.check_blas on my computer (Windows 8.1 64 bits) and it works (for both Python 2 and 3).

These are the commands executed:

For Python 2:

conda create --yes -n python2test python=2
activate python2test
conda install --yes numpy scipy mkl-service nose sphinx pydot-ng m2w64-gcc libpython
pip install git+https://github.com/mingwandroid/Theano.git@conda-fixes
python -m theano.misc.check_blas > log-python2.log 2>&1

python -V
rem Python 2.7.12 :: Continuum Analytics, Inc.
python -c "import platform; print (platform.platform())"
rem Windows-8.1-6.3.9600
python -c "import theano; print(theano.__version__)"
rem 0.9.0dev4.dev-c18e654db3b38c8d31b2cb5d94367e28d62f44aa

deactivate
conda remove --yes --all -n python2test

For Python 3:

conda create --yes -n python3test python=3
activate python3test
conda install --yes numpy scipy mkl-service nose sphinx m2w64-gcc libpython
pip install pydot
pip install git+https://github.com/mingwandroid/Theano.git@conda-fixes
python -m theano.misc.check_blas > log-python3.log 2>&1

python -V
rem Python 3.5.2 :: Continuum Analytics, Inc.
python -c "import platform; print (platform.platform())"
rem Windows-8.1-6.3.9600-SP0
python -c "import theano; print(theano.__version__)"
rem 0.9.0dev4.dev-c18e654db3b38c8d31b2cb5d94367e28d62f44aa

deactivate
conda remove --yes --all -n python3test

Some remarks:

On my computer I need to install m2w64-gcc package. Default conda package mingw does not work.
I had installed the package mkl-service to prevent a warning (not an error) that theano prints when running theano.misc.check_blas:

GTXWARNING (theano.configdefaults): mkl is not available, if using conda: `conda install mkl-service`: No module named mkl

@nouiz

mingwandroid · 2016-11-28T19:30:23Z

Thanks for checking.

mingw package is deprecated. It never supported 64-bit. We use mingw-w64 packages from MSYS2 instead.

All those conda packages you mention are of course listed as dependencies for the theano package so end-users need only to install that.

mingwandroid · 2016-11-28T19:32:15Z

Oh, m2w64-toolchain is recommended instead of m2w64-gcc.

mingwandroid · 2016-11-28T22:20:34Z

I do not have winpython 3.5 installed, but if you wish, I can try to take a look?

nouiz · 2016-11-29T14:01:36Z

Travis have some errors. Some some of them:

https://travis-ci.org/Theano/Theano/jobs/179590558#L532
https://travis-ci.org/Theano/Theano/jobs/179590558#L572

nouiz · 2016-11-29T14:03:49Z

theano/configdefaults.py

+        else:
+            # This branch is executed if no exception was raised
+            if sys.platform == "win32":
+                lib_path = os.path.join(sys.prefix, 'Library', 'bin')


On windows, lib_path is a list and otherwise, it is a list. I thin it should always be a list, so wrap this in a list here.

nouiz · 2016-11-29T14:04:04Z

theano/configdefaults.py

+                          blas_info.get('library_dirs', [])])
+            res = try_blas_flag(flags)
+            if res:
+                maybe_add_to_os_environ_pathlist('PATH', lib_path)


Now that lib_path will always be a list, use lib_path[0]

sys.prefix/DLLs for MKL DLLs

Many Windows users avoid the command-line and are unfamiliar with env. vars. They can elect to use Anaconda/Miniconda Python as their system Python and in that case would not be able to take advantage of g++ or mkl. This commit works around these issues by adding the appropriate values to os.environ['PATH'] if they are not already on PATH.

Since each of Anaconda/Miniconda/conda-forge change sys.version to contain different things, and some users may install MKL outside of conda, always try the import and then, if using conda, give some useful information on how to install MKL (by installing `mkl-service` not `mkl`).

mingwandroid · 2016-11-30T18:33:06Z

Is this ready to go? I believe I've addressed all the change requests? Thanks.

abergeron · 2016-11-30T19:47:44Z

theano/configdefaults.py

+# Anaconda on Windows has mingw-w64 packages including GCC, but it may not be on PATH.
+if rc != 0:
+    if sys.platform == "win32":
+        mingw_w64_gcc = os.path.join(os.path.dirname(sys.executable), "Library", "mingw-w64", "bin", "g++")


Is this supported for 32-bits?

Yes. That supports both no problem.

mingw-w64 is just the project name to differentiate it from mingw.org.

abergeron · 2016-11-30T19:47:55Z

theano/configdefaults.py

+        try:
+            rc = call_subprocess_Popen([mingw_w64_gcc, '-v'])
+            if rc == 0:
+                maybe_add_to_os_environ_pathlist('PATH', os.path.dirname(mingw_w64_gcc))


Also, the value of the cxx config param is a full path, so we can set it to the full path to g++ here and not fiddle with PATH.

I need to add to PATH I am afraid, otherwise companion tools (linker, cc1) are not found.

abergeron · 2016-11-30T19:54:52Z

theano/configdefaults.py

+        # Add sys.prefix/lib to the runtime search path. On
+        # non-system installations of Python that use the
+        # system linker, this is generally neccesary.
+        if sys.platform in ("linux", "darwin"):


If it does help on mac, that's great. But from what I observed I doubt it. We should test that.

abergeron · 2016-11-30T22:04:58Z

I'm basically ok with everything. We should just test the mkl part on mac to see if it works.

abergeron

I don't know why it works on mac (because everything I know tells me it shouldn't), but it does, so I'm ok.

nouiz · 2016-11-30T22:50:48Z

Thanks for all the work. I think all is good for this PR.

This week we should do a beta and not a release candidate. We want to make a release this week and there is probably too much stuff to finish this week, so we will go for a beta.

I lost the status of the conda packages of Theano and libgpuarray. Is there still work to do for them? thanks.

mingwandroid · 2016-11-30T22:58:32Z

Thanks for the reviews. Conda-wise I am in pretty good shape now I think. Everything appears to be working and all my patches are upstreamed.

I don't have an Nvidia GPU nor OpenCL capable AMD so that side of things I cannot test so easily which is a shame. When you are ready for the beta I will build new packages and if you were able to test them out on such hardware/drivers then that would be great.

I did notice a good few warnings from libgpuarray on Windows and some looked like they warranted more investigation so when I rebuild them I will check into that and send logs to you both.

nouiz requested changes Nov 28, 2016

View reviewed changes

This was referenced Nov 28, 2016

Theano 0.8.1 ImportError in Winpython 3.5.1 64bit, no GPU #4664

Closed

Conda fixes #5207

Closed

mingwandroid force-pushed the conda-fixes branch from c18e654 to dfdcfb2 Compare November 28, 2016 22:16

nouiz reviewed Nov 29, 2016

View reviewed changes

nouiz added this to the 0.9 milestone Nov 29, 2016

mingwandroid added 5 commits November 29, 2016 23:08

Conda: Search in sys.prefix/Library/bin, not

8d4bcf0

sys.prefix/DLLs for MKL DLLs

Win32: Allow Python 3.5

85af1b2

Conda: Update documentation

dbc63c9

mingwandroid force-pushed the conda-fixes branch from dfdcfb2 to f257350 Compare November 29, 2016 23:08

abergeron reviewed Nov 30, 2016

View reviewed changes

abergeron approved these changes Nov 30, 2016

View reviewed changes

nouiz approved these changes Nov 30, 2016

View reviewed changes

nouiz merged commit 08b6405 into Theano:master Nov 30, 2016

ColCarroll mentioned this pull request Dec 3, 2016

Add mkl-service to build pymc-devs/pymc#1572

Merged

lamblin mentioned this pull request Dec 16, 2016

Anaconda/Windows documentation is out of date. #5202

Closed

nouiz mentioned this pull request Aug 30, 2017

No ldflags and no fallback for [sd]gemv_ on Windows #6365

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Conda fixes #5290

Conda fixes #5290

mingwandroid commented Nov 25, 2016 •

edited

mingwandroid commented Nov 25, 2016

nouiz left a comment

nouiz Nov 28, 2016

abergeron Nov 30, 2016

nouiz Nov 28, 2016

mingwandroid Nov 28, 2016

nouiz Nov 28, 2016

notoraptor commented Nov 28, 2016

mingwandroid commented Nov 28, 2016

mingwandroid commented Nov 28, 2016

mingwandroid commented Nov 28, 2016

nouiz commented Nov 29, 2016

nouiz Nov 29, 2016

nouiz Nov 29, 2016

mingwandroid commented Nov 30, 2016

abergeron Nov 30, 2016

mingwandroid Nov 30, 2016

abergeron Nov 30, 2016

abergeron Nov 30, 2016

mingwandroid Nov 30, 2016

abergeron Nov 30, 2016

abergeron Nov 30, 2016

abergeron commented Nov 30, 2016

abergeron left a comment

nouiz commented Nov 30, 2016

mingwandroid commented Nov 30, 2016

		@@ -31,10 +31,6 @@

		import sys

		if sys.platform == 'win32' and sys.version_info[0:2] == (3, 5):

Conda fixes #5290

Conda fixes #5290

Conversation

mingwandroid commented Nov 25, 2016 • edited

mingwandroid commented Nov 25, 2016

nouiz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

notoraptor commented Nov 28, 2016

For Python 2:

For Python 3:

mingwandroid commented Nov 28, 2016

mingwandroid commented Nov 28, 2016

mingwandroid commented Nov 28, 2016

nouiz commented Nov 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mingwandroid commented Nov 30, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abergeron commented Nov 30, 2016

abergeron left a comment

Choose a reason for hiding this comment

nouiz commented Nov 30, 2016

mingwandroid commented Nov 30, 2016

mingwandroid commented Nov 25, 2016 •

edited