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

[WIP] julia 0.7 #41499

Merged
merged 0 commits into from Jul 31, 2018
Merged

[WIP] julia 0.7 #41499

merged 0 commits into from Jul 31, 2018

Conversation

nico202
Copy link
Contributor

@nico202 nico202 commented Jun 5, 2018

Motivation for this change

Julia 0.7 is going to be relased soon (R). Since it will bring Pkg3, it will be easier to be able to integrate the package management in nixos. With this attempt I hope to bring julia 0.7-alpha to a good PR state so that upgrading to the released version will be easy. So please tell me how to fix the PR to a decent state, then I'll squash things waiting for the day 0.7 will be officially released

Things done

I upgraded the used llvm to 6.0 (suggested version). I'm applying llvm patches as here, execept one that is needed for mingw32, and 2 that are breaking llvm unit tests. If anybody knows why is welcome.
In julia tests are working, except for 2 that requires network connection (but network presence is checked with getipaddr() that seems to work during tests, even if there's no network). The only other failing test is Distributed but I have to figure out why (it depends on Socket maybe is related to the network).

In this PR i'd also like to drop old julia versions (except for 0.6.2, that must be upgraded to 0.6.3).

I think that patches should be fetched with fetchPatch, but I want a feedback before start messing (compiling it takes like forever so I prefer to do changes only when I'm sure to do the right thing)

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@7c6f434c
Copy link
Member

7c6f434c commented Jun 6, 2018

Julia part seems fine to me.

I have not read LLVM patches; maybe @dtzWill @lovek323 @Ralith @Ericson2314 @matthewbauer might have some opinion on them.

Copy link

@nalimilan nalimilan left a comment

Choose a reason for hiding this comment

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

Really cool, thanks!

sha256 = "0s6dik373yvpj8xxmps3wasr6s5wrzbccpfw7vzlclbgxlpyjq0i";
};

rmathVersion = "0.1";

Choose a reason for hiding this comment

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

Rmath isn't needed anymore for Julia 0.7.

, libunwind, readline, utf8proc, zlib
, llvm, libffi, ncurses
# standard library dependencies
, curl, fftwSinglePrec, fftw, gmp, libgit2, mpfr, openlibm, openspecfun, pcre2

Choose a reason for hiding this comment

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

openspecfun isn't needed anymore for Julia 0.7.

march = { "x86_64" = "x86-64"; "i686" = "pentium4"; }."${arch}"
or (throw "unsupported architecture: ${arch}");
# Julia requires Pentium 4 (SSE2) or better
cpuTarget = { "x86_64" = "x86-64"; "i686" = "pentium4"; }."${arch}"

Choose a reason for hiding this comment

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

You don't actually need to set CPU target if it's identical to MARCH. However, it would be a good idea to set it to something more fancy so that several images are built, to enable the use of recent instructions where possible (this matters a lot for scientific work). Upstream uses this:

  • x86: 'JULIA_CPU_TARGET="pentium4;sandybridge,-xsaveopt,clone_all" '
  • x86_64: 'JULIA_CPU_TARGET="generic;sandybridge,-xsaveopt,clone_all;haswell,-rdrnd,base(1)'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@7c6f434c I've taken it from your commit, do you know why/if it's still required? I don't like messing with this because years back I had non-deterministic builds (because of SSE2 optimizations #13662)

Choose a reason for hiding this comment

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

Yeah, but Julia 0.7 is much more robust now, it will always use SSE2 and it will throw an error if compiler options are incorrect. Since the JULIA_CPU_TARGET values I posted come from upstream, they should be safe to use.

Copy link
Member

Choose a reason for hiding this comment

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

I think it was way before 0.7, though, if there are improvements. we could try being optimistic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So like this?

      cpuTarget = { "x86_64" = "generic;sandybridge,-xsaveopt,clone_all;haswell,-rdrnd,base(1)";
                    "i686" = "pentium4;sandybridge,-xsaveopt,clone_all"; }."${arch}"

(Compiling)


"USE_SYSTEM_BLAS=1"
"USE_BLAS64=${if openblas.blas64 then "1" else "0"}"
"LIBBLAS=-lopenblas"

Choose a reason for hiding this comment

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

Do you know is this is the threaded OpenBLAS? I'm asking because I remember it wasn't completely obvious which library to use when I created the Fedora package.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's compiled with

      ''PREFIX="''$(out)"''
      "NUM_THREADS=64"
      "INTERFACE64=${if blas64 then "1" else "0"}"
      "NO_STATIC=1"

Choose a reason for hiding this comment

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

I'm not sure but USE_THREAD=1 might be needed too. It's easy to check: if you run a linear algebra operation on a large matrix, more than 1 CPU will be used if threads are enabled.

FWIW, the Fedora package builds three variants of OpenBLAS: one without threads, one with threads, one with OpenMP. https://src.fedoraproject.org/rpms/openblas/blob/master/f/openblas.spec

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@. rand(10000,10000)^11 * rand(10000,10000)

100% on all 4 CPUs

"USE_SYSTEM_PCRE=1"
"PCRE_CONFIG=${pcre2.dev}/bin/pcre2-config"
"PCRE_INCL_PATH=${pcre2.dev}/include/pcre2.h"
"USE_SYSTEM_READLINE=1"

Choose a reason for hiding this comment

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

This option has been removed in 0.7.

@nico202
Copy link
Contributor Author

nico202 commented Jun 6, 2018

@nalimilan Thanks, compiling. I applied all of those except the cpu thing for now

@Bvermaa I'll add c548814 too

@nico202
Copy link
Contributor Author

nico202 commented Jun 6, 2018

Things done:

Compiled with tests and run binaries

Passing version info (waiting for answer on c548814)

@rened
Copy link

rened commented Jun 6, 2018

Thanks for tackling this! The Linux version builds and starts fine, on osx 10.13.5 I got the following:

make[3]: Entering directory '/private/tmp/nix-build-llvm-6.0.0.drv-0/llvm/build'
[100%] Linking CXX executable ../../bin/yaml2obj
cd /tmp/nix-build-llvm-6.0.0.drv-0/llvm/build/tools/yaml2obj && /nix/store/9nr9323rdvl9lbjkxz8lkahp1n8g69hn-cmake-3.11.2/bin/cmake -E cmake_link_script CMakeFiles/yaml2obj.dir/link.txt --verbose=y
/nix/store/2wc9z9plpwwmy1ck3g5wl34brcknvzp4-clang-wrapper-5.0.2/bin/clang++   -stdlib=libc++ -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -std=c++11 -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -O3 -DNDEBUG -arch x86_64 -Wl,-search_paths_first -Wl,-headerpad_max_install_names  -stdlib=libc++  -Wl,-dead_strip CMakeFiles/yaml2obj.dir/yaml2obj.cpp.o CMakeFiles/yaml2obj.dir/yaml2coff.cpp.o CMakeFiles/yaml2obj.dir/yaml2elf.cpp.o CMakeFiles/yaml2obj.dir/yaml2macho.cpp.o CMakeFiles/yaml2obj.dir/yaml2wasm.cpp.o  -o ../../bin/yaml2obj ../../lib/libLLVM.dylib
make[3]: Leaving directory '/private/tmp/nix-build-llvm-6.0.0.drv-0/llvm/build'
[100%] Built target yaml2obj
make -f CMakeFiles/check-all.dir/build.make CMakeFiles/check-all.dir/depend
make[3]: Entering directory '/private/tmp/nix-build-llvm-6.0.0.drv-0/llvm/build'
cd /tmp/nix-build-llvm-6.0.0.drv-0/llvm/build && /nix/store/9nr9323rdvl9lbjkxz8lkahp1n8g69hn-cmake-3.11.2/bin/cmake -E cmake_depends "Unix Makefiles" /tmp/nix-build-llvm-6.0.0.drv-0/llvm /tmp/nix-build-llvm-6.0.0.drv-0/llvm /tmp/nix-build-llvm-6.0.0.drv-0/llvm/build /tmp/nix-build-llvm-6.0.0.drv-0/llvm/build /tmp/nix-build-llvm-6.0.0.drv-0/llvm/build/CMakeFiles/check-all.dir/DependInfo.cmake --color=
Dependee "/tmp/nix-build-llvm-6.0.0.drv-0/llvm/build/CMakeFiles/check-all.dir/DependInfo.cmake" is newer than depender "/tmp/nix-build-llvm-6.0.0.drv-0/llvm/build/CMakeFiles/check-all.dir/depend.internal".
Dependee "/tmp/nix-build-llvm-6.0.0.drv-0/llvm/build/CMakeFiles/CMakeDirectoryInformation.cmake" is newer than depender "/tmp/nix-build-llvm-6.0.0.drv-0/llvm/build/CMakeFiles/check-all.dir/depend.internal".
Scanning dependencies of target check-all
make[3]: Leaving directory '/private/tmp/nix-build-llvm-6.0.0.drv-0/llvm/build'
make -f CMakeFiles/check-all.dir/build.make CMakeFiles/check-all.dir/build
make[3]: Entering directory '/private/tmp/nix-build-llvm-6.0.0.drv-0/llvm/build'
[100%] Running all regression tests
/nix/store/q3ncxyf9ql0wffblk758a2790mjfvan6-python-2.7.15/bin/python2.7 /tmp/nix-build-llvm-6.0.0.drv-0/llvm/build/./bin/llvm-lit -sv /tmp/nix-build-llvm-6.0.0.drv-0/llvm/build/utils/lit /tmp/nix-build-llvm-6.0.0.drv-0/llvm/build/test
llvm-lit: /tmp/nix-build-llvm-6.0.0.drv-0/llvm/build/utils/lit/tests/lit.cfg:61: warning: Could not import psutil. Some tests will be skipped and the --timeout command line argument will not work.
llvm-lit: /tmp/nix-build-llvm-6.0.0.drv-0/llvm/utils/lit/lit/TestingConfig.py:101: fatal: unable to parse config file '/tmp/nix-build-llvm-6.0.0.drv-0/llvm/test/lit.cfg.py', traceback: Traceback (most recent call last):
  File "/tmp/nix-build-llvm-6.0.0.drv-0/llvm/utils/lit/lit/TestingConfig.py", line 88, in load_from_path
    exec(compile(data, path, 'exec'), cfg_globals, None)
  File "/tmp/nix-build-llvm-6.0.0.drv-0/llvm/test/lit.cfg.py", line 277, in <module>
    result = sysctl_cmd.stdout.read().decode('ascii')
NameError: name 'sysctl_cmd' is not defined

Could not exec sysctl
make[3]: *** [CMakeFiles/check-all.dir/build.make:58: CMakeFiles/check-all] Error 2
make[3]: Leaving directory '/private/tmp/nix-build-llvm-6.0.0.drv-0/llvm/build'
make[2]: *** [CMakeFiles/Makefile2:291: CMakeFiles/check-all.dir/all] Error 2
make[2]: Leaving directory '/private/tmp/nix-build-llvm-6.0.0.drv-0/llvm/build'
make[1]: *** [CMakeFiles/Makefile2:298: CMakeFiles/check-all.dir/rule] Error 2
make[1]: Leaving directory '/private/tmp/nix-build-llvm-6.0.0.drv-0/llvm/build'
make: *** [Makefile:251: check-all] Error 2
builder for '/nix/store/4qgf66paq2paidlrhxxjynxvhpy3pc4a-llvm-6.0.0.drv' failed with exit code 2
cannot build derivation '/nix/store/fj8vrq8w1inlr8a4i603ml2i34j3y710-julia-0.7.0.drv': 1 dependencies couldn't be built
error: build of '/nix/store/fj8vrq8w1inlr8a4i603ml2i34j3y710-julia-0.7.0.drv' failed

If this is still expected please feel free to ping me once/if you need osx testers.

@nico202
Copy link
Contributor Author

nico202 commented Jun 6, 2018

@rened thanks for trying it out! (un)fortunately I do not own a mac so I cannot test it. However I'm just applying julia patches, can you try and verify llvm-6.0 alone (ie unpatched) does work? Thanks

@rened
Copy link

rened commented Jun 6, 2018

@nico202 as far as I can see that worked, yes

@rened
Copy link

rened commented Jun 6, 2018

@nico202 building julia now with the review changes included, let's see

@nico202
Copy link
Contributor Author

nico202 commented Jun 6, 2018

It shouln't change since I've not touched llvm. I've looked at the patches but nothing seems to relate to python/sysctl. You could try disabling all the pachets (from file all-packags.nix line 6682 on), compile llmv6, if it works enable one more and check again.

@rened
Copy link

rened commented Jun 6, 2018

will do!

@rened
Copy link

rened commented Jun 7, 2018

@nico202 I simply had to disable doCheck =true in llvm_6.overrideDerivation - the llvm package has doCheck = stdenv.isLinux && (!stdenv.isi686);, so testing does not seem to be supported on darwin.

The build worked and julia starts!

@nico202
Copy link
Contributor Author

nico202 commented Jun 7, 2018

Good to know, thanks! I removed the override.

I replaced patches with fetchpatch that is the suggested method (it's ugly to have all of them in all-packages.nix, any suggestion?)

I also updated julia-git to actually be the latest git version as of now. It's compiling fine too

edit: removed version 0.5

@rened
Copy link

rened commented Jun 7, 2018

Great! Testing all three versions on osx and Ubuntu

@nico202
Copy link
Contributor Author

nico202 commented Jun 7, 2018

(the git version is probably failing. 0.6 should be upgraded to 0.6.3 in theory but it's not my prio right now)

@rened
Copy link

rened commented Jun 7, 2018

The hash of one of the patches seems to be wrong:

trying https://raw.githubusercontent.com/JuliaLang/julia/v0.7.0-alpha/deps/patches/llvm-6.0-r327540.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  3383  100  3383    0     0  12082      0 --:--:-- --:--:-- --:--:-- 12082
fixed-output derivation produced path '/nix/store/wicgqg6jr1nb71jl3kzw1rwb02j9339d-llvm-6.0-r327540.patch' with sha256 hash '17xm0fa84py13iq8vyn4mvf7swnn6p3qv5sflwgv9crjpcydymfp' instead of the expected hash '092mmma5xrjx1hkd4my81jgl5v43d56pj31q9avfw0d2a6g8fi1v'
cannot build derivation '/nix/store/60qk9zn0pr5f2l11ax66djayrly1z86b-llvm-6.0.0.drv': 1 dependencies couldn't be built

@rened
Copy link

rened commented Jun 7, 2018

Hitting the next hash in 87d233c:

trying https://raw.githubusercontent.com/JuliaLang/julia/v0.7.0-alpha/deps/patches/llvm-6.0.0_D27296-libssp.patch
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  1592  100  1592    0     0   7075      0 --:--:-- --:--:-- --:--:--  7075
fixed-output derivation produced path '/nix/store/3y1jcfr1gywx4smp3p690y38dilmxdy4-llvm-6.0.0_D27296-libssp.patch' with sha256 hash '1c0hpcrn29gqkinkzhri8d486g05744r4pp0sw2yfc7mlhv8wc1v' instead of the expected hash '06z8rsg050mzr5qfgz53zz1cky203m4cswcs9l1v7n6vywns9ym5'
cannot build derivation '/nix/store/7jsz933dy0smz3gmqv92pajqmj6a2hyy-llvm-6.0.0.drv': 1 dependencies couldn't be built

@rened
Copy link

rened commented Jun 7, 2018

Suggestion, shouldn't we have juliastill mean 0.6.x and only switch once 0.7 is released, i.e. use julia_07 for now? Like this we could merge this PR as soon as the builds work.

@nico202
Copy link
Contributor Author

nico202 commented Jun 7, 2018

sure. Done and fixed hashes.

nix-env -f . -i julia # -> julia 0.6.2
nix-env -f . -i julia-0.7.0 # -> julia 0.7.0-alpha
nix-env -f . -i julia-git # -> julia 0.7.0-alpha.0

I'll re-build all of them on NixOS (will take a lot), If you can try again on mac+ubuntu we'll have a complete test and I could then squash everything

@rened
Copy link

rened commented Jun 7, 2018

All three builds succeed on darwin and Ubuntu - thanks again for your efforts, this is great!

@nico202
Copy link
Contributor Author

nico202 commented Jun 25, 2018

0.7.0-beta is out, I'm building, tomorrow I'll push the new commit

@rened
Copy link

rened commented Jul 31, 2018

@nico202, any progress? beta2 is out, rc1 should come today/tomorrow - shall we rerun the tests and commit? (cc @flicaflow)

@nico202
Copy link
Contributor Author

nico202 commented Jul 31, 2018

I upgraded to beta2, I can push it. I'll leave for vacation tomorrow so I wont update to rc1 before 2 weeks

@nico202 nico202 closed this Jul 31, 2018
@nico202
Copy link
Contributor Author

nico202 commented Jul 31, 2018

No idea of what's happening. There's here the alpha -> beta upgrade, https://github.com/nico202/nixpkgs/tree/julia07, not tested after the rebase on master though.

@dotlambda
Copy link
Member

I did definitely not merge this 😕

@rened
Copy link

rened commented Jul 31, 2018

@nico202 I will test it, but could you please use beta2 instead of beta?

@7c6f434c
Copy link
Member

7c6f434c commented Aug 1, 2018

@dotlambda I think there was a branch reset, and Github has some detection of manual merges, and their code detects every situation where you push a commit that is a descendant of all commits currently in the PR branch. If PR branch is reset to master, GitHub considers any push to master a merge of the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants