conda-forge / openblas-feedstock

A conda-smithy repository for openblas.
BSD 3-Clause "New" or "Revised" License
9 stars 38 forks source link

Build on Windows with Clang and Flang #34

Closed isuruf closed 6 years ago

isuruf commented 7 years ago

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

conda-forge-linter commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

There's a PR for DYNAMIC_ARCH here

scopatz commented 6 years ago

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 commented 6 years ago

@scopatz, do you need LAPACK or just blas?

scopatz commented 6 years ago

I think it needs LAPACK too

isuruf commented 6 years ago

Okay, then I need to fix https://github.com/flang-compiler/flang/pull/288 then which should take anything between 1 week to months

scopatz commented 6 years ago

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

ghost commented 6 years ago

@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 commented 6 years ago

conda-forge also has clapack

ghost commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

ghost commented 6 years ago

Yes but conda gets two hours.

ghost commented 6 years ago

Looks like the patch failed. I don't think the windows build will pass.

isuruf commented 6 years ago

Hmm, the patch applied successfully locally.

isuruf commented 6 years ago

This is ready now. Only needs a decision in https://github.com/conda-forge/openblas-feedstock/issues/2#issuecomment-346204519

scopatz commented 6 years ago

I don't see a reason not to have something in.

jakirkham commented 6 years ago

So based on the outcome of the meeting last week, it sounds like solution should be to use MinGW-w64 compilers.

That said, I do see some value on pushing on Flang as well. Just given Flang is in its pre-alpha days I'd be very hesitant to add something like this to conda-forge/main. However maybe we could do something like add Flang and Flang using packages to conda-forge/flang for instance. FWIW meant to mention this is the meeting, but we were short on time.

That way users have to choose this route, but do have the option. Also we could even go so far as to have a warning message printed by the flang runtime package (assuming we have one somewhere), which warns that all of this stuff is bleeding edge and not yet released.

Thoughts?

cc @msarahan @mingwandroid

isuruf commented 6 years ago

So based on the outcome of the meeting last week, it sounds like solution should be to use MinGW-w64 compilers.

From what I remember, this was never agreed upon. From previous discussions, compatibility with defaults was emphasized, which means only Intel Fortran compiler can be used. Neither MinGW-w64 nor Flang are compatible with defaults.

msarahan commented 6 years ago

That was not my understanding from @xoviat. He said that is ABI compatible with Intel and MSVC: https://github.com/conda-forge/conda-forge.github.io/issues/460#issuecomment-343297168

I have no idea what is true of MinGW-w64.

My main point here is not necessarily pure compatibility. My main point is: please don't implement something that makes it very easy for people to break. If there's some mutual exclusivity scheme we can both implement like blas/nomkl feature stuff (or perhaps not, because that doesn't seem to work exceedingly well), that's all I want. So long as the ecosystem isn't causing people pain, I'm happy. I'm mostly agnostic on how we prevent that pain.

ghost commented 6 years ago

Wait, flang should be compatible with Intel. Do you have evidence to the contrary?

isuruf commented 6 years ago

That was not my understanding from @xoviat. He said that is ABI compatible with Intel and MSVC: conda-forge/conda-forge.github.io#460 (comment)

With MSVC, sure. @xoviat, are you sure the fortran name-mangling is the same for both ifort and flang? If so, then I guess they are compatible

My main point here is not necessarily pure compatibility. My main point is: please don't implement something that makes it very easy for people to break. If there's some mutual exclusivity scheme we can both implement like blas/nomkl feature stuff (or perhaps not, because that doesn't seem to work exceedingly well), that's all I want. So long as the ecosystem isn't causing people pain, I'm happy. I'm mostly agnostic on how we prevent that pain.

That's why I suggested a feature, flang. These conda-forge packages may then be installed only if flang-meta (which tracks the feature flang) is installed as well. Otherwise defaults packages are installed.

ghost commented 6 years ago

With MSVC, sure. @xoviat, are you sure the fortran name-mangling is the same for both ifort and flang? If so, then I guess they are compatible.

Yes.

jakirkham commented 6 years ago

Would also be ok with a feature as opposed to a channel label. Though agree with @msarahan that features have not done a great job historically. Would be eager to see something better.

Would be nice if that metapackage did issue a warning from a post-link script though. Want to make sure users are aware this is bleeding edge and there may be surprises.

Side note: How are you sure about Flang's compatibility with ifort? Is that documented somewhere?

isuruf commented 6 years ago

Would also be ok with a feature as opposed to a channel label. Though agree with @msarahan that features have not done a great job historically. Would be eager to see something better.

Sorry, didn't see that part in Mike's comment. I'm not that into different channels, because it's very difficult to setup with conda-smithy. (Can conda-smithy generate a feedstock that can upload to different channels based on OS?)

ghost commented 6 years ago

It's untested but they use the same symbols and calling conventions AFAIK. There's no documentation for flang; we're the ones (along with NVIDIA) developing it.

msarahan commented 6 years ago

a flang feature is exactly the way I would not do it. Features are decent at keeping things grouped when they all have the same feature. What took me a long time to learn is that features are terrible at enforcing some kind of mutual exclusivity amongst a set of options. We've had headaches lately with the VC features because more than one package had a track_feature. That freed conda up to not have mutual exclusivity among VC features. Think of features more like a priority thing rather than an exclusivity thing. It's not that the vc14 feature will prevent vc9 things from being installed, it's more that conda will strongly prefer vc14 things where they are available. Things get really weird when more than one track_feature in a given set is active, and when nothing with an appropriate feature is available.

The metapackage approach allows mutual exclusivity much better, but it's very tedious and I haven't had enough experience with it yet.

In short, please DON'T use a simple "flang" feature, because it's a poor way to enforce exclusivity with what any other option might be.

isuruf commented 6 years ago

@msarahan, I'm clearly not familiar with conda internals. Can you help me out? What should I do?

ghost commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@msarahan, would you mind commenting on https://github.com/conda-forge/openblas-feedstock/pull/34#issuecomment-346431505 ?

msarahan commented 6 years ago

It's Thanksgiving. I'll be back on Monday.

On Nov 23, 2017 12:20, "Isuru Fernando" notifications@github.com wrote:

@msarahan https://github.com/msarahan, would you mind commenting on #34 (comment) https://github.com/conda-forge/openblas-feedstock/pull/34#issuecomment-346431505 ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/conda-forge/openblas-feedstock/pull/34#issuecomment-346678707, or mute the thread https://github.com/notifications/unsubscribe-auth/AACV-TjrB1MU7k8LeKp7LlTJTu5D5Knsks5s5bdqgaJpZM4OncRe .

jakirkham commented 6 years ago

Happy Thanksgiving Mike! 🦃

msarahan commented 6 years ago

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 commented 6 years ago

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

ghost commented 6 years ago

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

isuruf commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

Compare that to the current Windows development procedure!

mingwandroid commented 6 years ago

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

isuruf commented 6 years ago

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 commented 6 years ago

Did you confirm that build procedure?

ghost commented 6 years ago

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 commented 6 years ago

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