Skip to content
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

Build on Windows with Clang and Flang #34

Merged
merged 22 commits into from
Nov 29, 2017
Merged

Conversation

isuruf
Copy link
Member

@isuruf isuruf commented Jul 29, 2017

This build blas and cblas with clang, and lapack with flang.

@conda-forge-linter
Copy link

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 (recipe) and found it was in an excellent condition.

@jakirkham
Copy link
Member

Thanks for putting this together @isuruf!

Couple of thoughts. First, if we use VC 14, I guess that means we can't link Python 2.7 stuff with this. What would your proposal be in that case? Use m2w64-openblas? Second, if we go this route, I guess we can't support Fortran (yet). Any thoughts on how to handle things that want Fortran support? Admittedly this isn't an easy question. So maybe there is no good answer.

@isuruf
Copy link
Member Author

isuruf commented Jul 30, 2017

What would your proposal be in that case? Use m2w64-openblas?

No idea

Second, if we go this route, I guess we can't support Fortran (yet). Any thoughts on how to handle things that want Fortran support?

Wait until Flang is ready or use CLAPACK.

@isuruf
Copy link
Member Author

isuruf commented Aug 2, 2017

There's a PR for DYNAMIC_ARCH here

@scopatz
Copy link
Member

scopatz commented Nov 1, 2017

Any movement on this? This would be really great to have. I am looking to windows support for COIN, which also needs a BLAS...

@isuruf
Copy link
Member Author

isuruf commented Nov 1, 2017

@scopatz, do you need LAPACK or just blas?

@scopatz
Copy link
Member

scopatz commented Nov 1, 2017

I think it needs LAPACK too

@isuruf
Copy link
Member Author

isuruf commented Nov 1, 2017

Okay, then I need to fix flang-compiler/flang#288 then which should take anything between 1 week to months

@scopatz
Copy link
Member

scopatz commented Nov 1, 2017

Ho-kay! Thanks for letting me know! And thanks for working on these issues!

@ghost
Copy link

ghost commented Nov 1, 2017

@scopatz If you require an older LAPACK version (from 2009), you can just just the vcpkg BLAS. The performance is not good but it's something.

[powershell]
git clone https://github.com/microsoft/vcpkg.git
cd vcpkg
.\bootstrap.ps1
vcpkg install clapack:x64-windows

@isuruf
Copy link
Member Author

isuruf commented Nov 2, 2017

conda-forge also has clapack

@ghost
Copy link

ghost commented Nov 9, 2017

Is this ready to go (I assume using a patched cmake to build is okay for now, since it's only temporary), or will this be blocked due to the Python 2.7 issue?

@isuruf
Copy link
Member Author

isuruf commented Nov 9, 2017

I'm going to package cmake tomorrow. (CMake 3.10 is coming out tomorrow and I'll have the flang windows fixes on top of that).

Afterwards I need to figure out what patches are needed. DYNAMIC_ARCH PR, Flang PR and maybe others.

@ghost
Copy link

ghost commented Nov 9, 2017

If it were me, I'd just build OpenBLAS HEAD for windows only now, and then wait until the next version to release everything together. It's not ideal, but who knows what's in there that's enabling windows builds. It could be a mess (or it may not be).

@isuruf
Copy link
Member Author

isuruf commented Nov 10, 2017

Btw, appveyor's 1 hour limit is not enough to build with both DYNAMIC_ARCH=1 and NOFORTRAN=0

@ghost
Copy link

ghost commented Nov 10, 2017

Yes but conda gets two hours.

@ghost
Copy link

ghost commented Nov 22, 2017

If you're trying to make it difficult to install Intel components when using flang, that's not being helpful. There is no reason to restrict this. Possibly it would be helpful to restrict having both MKL and openblas.

@msarahan
Copy link
Member

Any exercise in restricting what gets installed with what is really not worthwhile until we've tested packages in mixed environments and run some test suites to see if anything breaks. I'm in agreement with @xoviat that we shouldn't arbitrarily restrict flang-compiled stuff to not coexist with intel-compiled stuff until we have evidence that it doesn't work.

@isuruf
Copy link
Member Author

isuruf commented Nov 22, 2017

So, a flang feature would install flang compiled packages from conda-forge, but by default ifort compiled packages from defaults is preferred. Isn't that what we want? Am I missing something?

@isuruf
Copy link
Member Author

isuruf commented Nov 22, 2017

Btw, the issue with vc is not applicable here, as we won't be adding flang-meta to the run requirements. (Adding vc to run requirements was the issue, but if it weren't added, then packages with the feature can't be installed without explicitly installing vc package. We are in luck here because that's exactly what we want. No flang compiled packages unless flang-meta is installed.)

@isuruf
Copy link
Member Author

isuruf commented Nov 23, 2017

@msarahan, would you mind commenting on #34 (comment) ?

@msarahan
Copy link
Member

msarahan commented Nov 23, 2017 via email

@jakirkham
Copy link
Member

Happy Thanksgiving Mike! 🦃

@msarahan
Copy link
Member

So, a flang feature would install flang compiled packages from conda-forge, but by default ifort compiled packages from defaults is preferred. Isn't that what we want? Am I missing something?

The key is always having a self-consistent set of software that is compatible within itself. With the mess we've seen with features, I am wary of any new scheme to restrict sets of software until I have seen packages or test cases to prove theories about how it might work. As @xoviat pointed out above, it's not even completely clear that we need to restrict Flang-built software from ifort-built software. We should test that by trying to identify two fortran-built packages that we can mix in an environment, and run the test suites for both of those packages, hoping to catch any potential incompatibilities.

Scipy comes to mind, but I'm not aware of many (any?) other major fortran-using software. Can any of you suggest something? Perhaps Openblas compiled with flang, scipy compiled with ifort, and vice-versa, just for good measure? If you need our help making packages compiled with ifort to test, we'll be happy to assist.

@ghost
Copy link

ghost commented Nov 27, 2017

Is there any way to merge this as a pre release so that we can test it?

@ghost
Copy link

ghost commented Nov 27, 2017

Actually I compiled scipy with flang+mkl and it worked for the most part except for three failures caused by be flang bugs.

This reverts commit a62ce4a.
@isuruf
Copy link
Member Author

isuruf commented Nov 28, 2017

As @xoviat said scipy compiled with flang+mkl almost works. I don't have access to ifort. @msarahan would you be able to test scipy compiled with ifort+openblas from this PR? (I think Intel is offering free licenses to open source contributors, I can check that out)

Btw, I've reverted the flang feature. Since this openblas is not interfering with defaults because there is no openblas for windows on defaults, any objections to merging ?

@isuruf
Copy link
Member Author

isuruf commented Nov 28, 2017

@xoviat, what errors did you get? If you give me instructions I can try it out and see if there's anything I can do. (I've installed mkl using conda)

@ghost
Copy link

ghost commented Nov 28, 2017

Just install my flang numpy branch:

pip install git+https://GitHub.com/xoviat/numpy.git@flang

then install scipy

pip install git+https//GitHub.com/scipy/scipy.git

Note that you must be in a conda environment with flang and mkl.

@ghost
Copy link

ghost commented Nov 28, 2017

Compare that to the current Windows development procedure!

@mingwandroid
Copy link

You still need visual studio installed though here do you not?

@isuruf
Copy link
Member Author

isuruf commented Nov 28, 2017

You still need visual studio installed though here do you not?

Yeah, but for development and packaging into conda, wheels, it's easier.

@conda-forge/openblas, any more comments on the PR?

@ghost
Copy link

ghost commented Nov 28, 2017

Did you confirm that build procedure?

@ghost
Copy link

ghost commented Nov 28, 2017

Actually it would be very nice if this was merged right now. I cannot upload the wheels that I built using MKL because it's too large (100MB!).

@ghost
Copy link

ghost commented Nov 28, 2017

Trying to work around a missing conda OpenBLAS is just painful, considering that OpenBLAS takes over an hour to build.

@scopatz
Copy link
Member

scopatz commented Nov 28, 2017

I think that this should go in if it can since it seems to be compatible with VS. I don't have merge rights, though, myself.

@ghost
Copy link

ghost commented Nov 28, 2017

@isuruf has merge rights, but of course may be worried about objections from others. Maybe wait another day, and merge if no more comments?

@scopatz
Copy link
Member

scopatz commented Nov 28, 2017

Sounds reasonable to me

@isuruf
Copy link
Member Author

isuruf commented Nov 29, 2017

@xoviat, I ran the tests for scipy built with flang+mkl. 4 failed. 12768 passed. I'll report on the scipy issue tracker if I find anything.

@ghost
Copy link

ghost commented Nov 29, 2017

The fortranfile issue is probably not that important, but there's precision loss that's happening somewhere; this causes an optimization routine to fail. It would be good if that could be fixed (though it's probably a bit hairy).

@ghost
Copy link

ghost commented Nov 29, 2017

@isuruf Can you merge?

@ghost
Copy link

ghost commented Nov 29, 2017

@isuruf Ping.

@isuruf isuruf merged commit c38925e into conda-forge:master Nov 29, 2017
@isuruf isuruf deleted the windows branch November 29, 2017 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants