-
-
Notifications
You must be signed in to change notification settings - Fork 37
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
[WIP] Windows support #78
Conversation
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
@xoviat, do you know the reason for the failure? |
There are 2 problems
@xoviat, do you know if we can compile scipy with clang-cl.exe? |
Theoretically possible but the time commitment is a open ended. The question is why can I compile scipy but you can't. |
This is the error,
Internal compiler errors are hard to reproduce. |
Idea: compile numpy with vs2017 and numpy PR #9965. |
Then compile scipy with vs2017. |
Can |
Yes |
How do I use VS2017? |
Put os: Visual Studio 2017 in appveyor.yml |
Yes, but that doesn't mean numpy.distutils will pick up VS2017 |
On the latest Python 3.6 it will. On older versions it still will if vs 2015 is not found. |
Fixed in the last commit. See the commit message. Your logs helped a lot. Thanks. |
x86_amd64 is a 32-bit compiler producing amd64 code, while amd64 is a 64-bit compiler producing amd64 code. Latter gives a ICE while former does not
26/12056 failed for flang+openblas which is more than flang+mkl. I think OpenBLAS is not well tested in windows with MSVC. I'll look into running OpenBLAS test-suite. |
@conda-forge-admin, please rerender |
Hi @isuruf what's up on this ? Are there any important defects revealed by the testsuite here of could we ignore some ? |
Looking at the failures, they are things like computing decompositions, determinants, inversions, solving for matrices, etc. These are things that I would typically associate with LAPACK. Though could be wrong. Did not check the code of the failing tests. Assuming LAPACK is the culprit, seeing failures on Windows here isn't too surprising given the LAPACK part of the OpenBLAS code base is basically unused on Windows. Perhaps we should look into alternative LAPACK implementations? OpenBLAS 0.20.0 includes ReLAPACK support, which could be a nice option. Raised issue ( conda-forge/openblas-feedstock#43 ) to explore it. Another option would be to use our own build of Netlib's LAPACK. Other suggestions? |
Thanks for following up on this @jakirkham (and everyone working before on this), getting SciPy to work on Windows is really not an easy task :) I'll add some things I just dig hoping that maybe you find something useful there: SciPy has an official build "script" used for testing on AppVeyor here: https://github.com/scipy/scipy/blob/master/appveyor.yml They use OpenBLAS 0.2.20 and Their builds are currently passing here: https://ci.appveyor.com/project/scipy/scipy |
The SciPy "official" build outputs this after
while on conda-forge, it outputs this:
I have no idea why it finds it twice in conda-forge, and why the language is I've inspected the OpenBLAS pre-compiled package they are using, it's from SciPy wheels "channel", it's version 0.2.20, and seems to be using the standard LAPACK, not ReLAPACK, but I'm still not 100% sure on this. |
Hmm, from here I've found the source of the OpenBLAS binaries they are using: https://github.com/matthew-brett/build-openblas Here are the flags they use:
perhaps it's the case to compare this against the conda-forge build of OpenBLAS on Windows? |
Hmm, this failure indicate that it might actually be a problem with LAPACK configuration for complex numbers?
there many possibilities of complex numbers configuration in LAPACK from OpenBLAS (in |
It might be picking up LAPACKE, which are the C bindings to LAPACK. Just guessing though. |
I'm really keen to get this working, but I have no idea how I can help. How do I get started? I have never worked on a feedstock before. Sorry if this is not the correct place for these questions. |
@FSund try to start from here: https://conda-forge.org/docs/buildwin.html#local-testing You should be able to build it locally by cloning this recipe and using There's a chat here where you might find help: https://gitter.im/conda-forge/conda-forge.github.io |
@tadeu I did what you said, but I'm getting the following warnings/errors during building/testing
I'm using a VM from here, have installed Microsoft Build Tools for Visual Studio 2017, Miniconda, and all packages in
I had to adjust the path to Any idea what I'm doing wrong? |
It seems scipy is the deprecated MarkInfo objects. I'm afraid this needs to be addressed upstream. Meanwhile I suggest to pin |
Thanks. Perhaps I'm running into a new issue with one of the tests. Specifically this one, which makes python crash
More detailed logs:
Seems like this could be the same issue: scipy/scipy#5338 Here is my
which I added in
It gets applied right before the resource usage statistics is listed, so I think I did it correctly, but the patch itself might not be correctly formatted.
|
pytest should appear only on test:
requires:
- pytest <4
- mpmath Also ideally we should add a comment to a related upstream issue, so the pin can be taken out in the future when it scipy is made compatible with pytest 4. |
Nice find @FSund! It seems that patch was already released in version 1.1.0: scipy/scipy@ce36940 This PR is over an older version of SciPy (1.0.1), perhaps rebase into master to see what happens? |
I tried rebasing into master, but there were a lot of conflicts I an not sure how to resolve. |
Might be easier to merge with |
I tried merging (locally) now. It seems like something with the build setup must have changed, because I'm getting the following error during building which I didn't get before
It's looking for the registry key I will try setting the registry key manually and look for any further issues during build. |
I got past the TL;DR: Full traceback
|
I did some thinking, tried reinstalling VS Build Tools again, but nothing helped. Then I noticed the a DeprecationWarning just above the wall of text I posted above. Is the build failing on that warning, and not on the Here's the output from just before the lines I posted in my previous comment
Would the DeprecationWarning trigger a traceback and exiting the build? |
I just managed to build numpy 1.11.3 with the windows compilers on python 3.6+ conda-forge/numpy-feedstock#128 Maybe that recipe can be merge and we can try this again? |
@isuruf closing this as stale. I guess that if we are to try this again it is better to start fresh. |
Depends on conda-forge/numpy-feedstock#73