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.

Sign up for GitHub

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

Jump to bottom

Use new PackageCompiler and MKL_jll #29

Merged
merged 6 commits into from
Feb 23, 2020
Merged

Use new PackageCompiler and MKL_jll #29

merged 6 commits into from
Feb 23, 2020

Conversation

KristofferC
Copy link
Contributor

@KristofferC KristofferC commented Feb 14, 2020 •
edited
Loading

The new strategy is to instead of setting Base.libmkl to the full path we just set it to MKL_jll.libmkl_rt and then make sure that we load MKL and MKL_jll (and run their __init__) before LinearAlgrebra so that everything is nicely set up when LinearAlgebra determines vendor etc.

Fixes #24, fixes #27

@KristofferC KristofferC force-pushed the kc/mkl_jll_packagecompiler branch 4 times, most recently from 7b442ef to 80de594 Compare February 14, 2020 14:37
@KristofferC KristofferC force-pushed the kc/mkl_jll_packagecompiler branch 2 times, most recently from 37f85e4 to 0b8cf0b Compare February 14, 2020 15:34
@KristofferC
Copy link
Contributor Author

KristofferC commented Feb 14, 2020 •
edited
Loading

I think this should work now. Would be grateful for any testing.

Also, we can disable AV since I made travis test Windows.

@KristofferC KristofferC changed the title WIP: use new PackageCompiler and MKL_jll Use new PackageCompiler and MKL_jll Feb 14, 2020
@daviehh daviehh mentioned this pull request Feb 14, 2020
Closed
@aminya
Copy link
Contributor

aminya commented Feb 14, 2020 •
edited
Loading

Appveyor has trouble building Windows, so I added a PR to this branch using Github actions.
#30

@KristofferC
Copy link
Contributor Author

Also, we can disable AV since I made travis test Windows.

(I also deleted the .appveyor file which is why AppVeyor fails).

staticfloat
staticfloat approved these changes Feb 14, 2020
View reviewed changes
Copy link
Member

@staticfloat staticfloat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me

@aminya
Copy link
Contributor

aminya commented Feb 14, 2020

Also, we can disable AV since I made travis test Windows.

(I also deleted the .appveyor file which is why AppVeyor fails).

If Appveyor is going to throw errors without a yml, we should remove their app from the repository.

@aminya
Copy link
Contributor

aminya commented Feb 14, 2020

Also fixes #27

@KristofferC
Copy link
Contributor Author

If Appveyor is going to throw errors without a yml, we should remove their app from the repository.

Yeah, I think only @andreasnoack can do that since from the URL it looks like it is on his account

@andreasnoack
Copy link
Member

I've deleted MKL.jl from AppVeyor

aminya
aminya approved these changes Feb 16, 2020
View reviewed changes
Copy link
Contributor

@aminya aminya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on windows 10 and Julia 1.5

@KristofferC KristofferC mentioned this pull request Feb 16, 2020
Closed
aminya
aminya reviewed Feb 23, 2020
View reviewed changes
.travis.yml Outdated
email: false

script:
julia --project="" -e 'using Pkg; Pkg.add(PackageSpec(name ="PackageCompiler", rev="master"))';
Copy link
Contributor

@aminya aminya Feb 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
julia --project="" -e 'using Pkg; Pkg.add(PackageSpec(name ="PackageCompiler", rev="master"))';

Now that PackageCompiler is registered this can be updated

@aminya
Copy link
Contributor

aminya commented Feb 23, 2020

@KristofferC Before merging this to Master please merge #30 first. Thank you!

aminya and others added 2 commits February 23, 2020 09:07
@aminya
GitHub actions (#30)
e7ae276 
* Adding workflow files

* Adding Github actions CI

* Delete ci.yml
@KristofferC
no longer check out master PackageCompiler in .travis
dc300dd
@KristofferC KristofferC merged commit 61720ba into master Feb 23, 2020
@delete-merged-branch delete-merged-branch bot deleted the kc/mkl_jll_packagecompiler branch February 23, 2020 08:42
@AzamatB AzamatB mentioned this pull request Feb 23, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aminya aminya aminya approved these changes

@staticfloat staticfloat staticfloat approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Using new PackageCompiler MKL isn't built on Windows
4 participants
@KristofferC @aminya @andreasnoack @staticfloat